Fix crossover (#212)

* Fix equivalent subtree, add test

* Fix crossover

* Fix mol adapter

* Minor

* Review fixes

examples/molecule_search/mol_adapter.py

@@ -17,6 +17,8 @@ def __init__(self):
 
     def _restore(self, opt_graph: OptGraph, metadata: Optional[Dict[str, Any]] = None) -> MolGraph:
         digraph = self.nx_adapter.restore(opt_graph)
+        # return to previous node indexing
+        digraph = nx.relabel_nodes(digraph, dict(digraph.nodes(data='nxid')))
         digraph = restore_edges_params_from_nodes(digraph)
         nx_graph = digraph.to_undirected()
         mol_graph = MolGraph.from_nx_graph(nx_graph)
@@ -25,19 +27,21 @@ def _restore(self, opt_graph: OptGraph, metadata: Optional[Dict[str, Any]] = Non
     def _adapt(self, adaptee: MolGraph) -> OptGraph:
         nx_graph = adaptee.get_nx_graph()
         digraph = nx_to_directed(nx_graph)
+        digraph = store_edges_params_in_nodes(digraph)
         opt_graph = self.nx_adapter.adapt(digraph)
-        opt_graph = store_edges_params_in_nodes(digraph, opt_graph)
         return opt_graph
 
 
-def store_edges_params_in_nodes(graph: nx.DiGraph, opt_graph: OptGraph) -> OptGraph:
+def store_edges_params_in_nodes(graph: nx.DiGraph) -> nx.DiGraph:
     graph = deepcopy(graph)
+    edges_params = {}
     for node in graph.nodes():
-        edges_params = {}
+        edge_params = {}
         for predecessor in graph.predecessors(node):
-            edges_params.update({str(predecessor): graph.get_edge_data(predecessor, node)})
-        opt_graph.get_node_by_uid(str(node)).parameters.update({'edges_params': edges_params})
-    return opt_graph
+            edge_params.update({str(predecessor): graph.get_edge_data(predecessor, node)})
+        edges_params.update({node: edge_params})
+    nx.set_node_attributes(graph, edges_params, name='edges_params')
+    return graph
 
 
 def restore_edges_params_from_nodes(graph: nx.DiGraph) -> nx.DiGraph:

examples/molecule_search/mol_graph.py

@@ -61,6 +61,8 @@ def get_nx_graph(self) -> nx.Graph:
 
         for atom in self._rw_molecule.GetAtoms():
             graph.add_node(atom.GetIdx(),
+                           # save indices in node data to restore edges parameters after adapt-restore process
+                           nxid=str(atom.GetIdx()),
                            name=atom.GetSymbol(),
                            atomic_num=atom.GetAtomicNum(),
                            formal_charge=atom.GetFormalCharge(),

golem/core/adapter/nx_adapter.py

@@ -48,7 +48,6 @@ def map_predecessors(node_id) -> Iterable[OptNode]:
         for node_id, node_data in adaptee.nodes.items():
             # transform node
             node = self._node_adapt(node_data)
-            node.uid = str(node_id)
             mapped_nodes[node_id] = node
 
         # map parent nodes

golem/core/optimisers/genetic/gp_operators.py

@@ -1,6 +1,6 @@
 import itertools
 from copy import deepcopy
-from typing import Any, List, Tuple
+from typing import Any, List, Tuple, Optional
 
 from golem.core.dag.graph_node import descriptive_id_recursive_nodes
 from golem.core.dag.graph_utils import distance_to_primary_level
@@ -15,8 +15,8 @@ def equivalent_subtree(graph_first: Any, graph_second: Any, with_primary_nodes:
 
     pairs_list = []
     all_nodes = graph_first.nodes + graph_second.nodes
-    all_descriptive_ids = [set(descriptive_id_recursive_nodes(node)) for node in graph_first.nodes] \
-                          + [set(descriptive_id_recursive_nodes(node)) for node in graph_second.nodes]
+    all_descriptive_ids = [set(descriptive_id_recursive_nodes(node)) for node in graph_first.nodes] +\
+                          [set(descriptive_id_recursive_nodes(node)) for node in graph_second.nodes]
     all_recursive_ids = dict(zip(all_nodes, all_descriptive_ids))
     for node_first in graph_first.nodes:
         for node_second in graph_second.nodes:
@@ -67,24 +67,34 @@ def filter_duplicates(archive, population) -> List[Any]:
     return filtered_archive
 
 
-def structural_equivalent_nodes(node_first: Any, node_second: Any, recursive_ids: dict = None) -> List[Tuple[Any, Any]]:
+def structural_equivalent_nodes(node_first: Any,
+                                node_second: Any,
+                                recursive_ids: Optional[dict] = None,
+                                seen: Optional[List[Any]] = None) -> List[Tuple[Any, Any]]:
     """ Returns the list of nodes from which subtrees are structurally equivalent.
     :param node_first: node from first graph from which to start the search.
     :param node_second: node from second graph from which to start the search.
     :param recursive_ids: dict with recursive descriptive id of node with nodes as keys.
+    :param seen: list of already visited nodes to avoid infinite recursion.
     Descriptive ids can be obtained with `descriptive_id_recursive_nodes`.
     """
 
     nodes = []
     is_same_type = type(node_first) == type(node_second)
+    seen = seen or []
+
+    if node_first in seen or node_second in seen:
+        return []
+    seen.append(node_first)
+    seen.append(node_second)
     # check if both nodes are primary or secondary
     if hasattr(node_first, 'is_primary') and hasattr(node_second, 'is_primary'):
         is_same_graph_node_type = node_first.is_primary == node_second.is_primary
         is_same_type = is_same_type and is_same_graph_node_type
 
     for node1_child, node2_child in itertools.product(node_first.nodes_from, node_second.nodes_from):
         nodes_set = structural_equivalent_nodes(node_first=node1_child, node_second=node2_child,
-                                                recursive_ids=recursive_ids)
+                                                recursive_ids=recursive_ids, seen=seen)
         nodes.extend(nodes_set)
     if is_same_type and len(node_first.nodes_from) == len(node_second.nodes_from) \
             and are_subtrees_the_same(match_set=nodes,

golem/core/optimisers/optimizer.py

@@ -113,7 +113,9 @@ def __init__(self,
                  graph_optimizer_params: Optional[AlgorithmParameters] = None):
         self.log = default_log(self)
         self._objective = objective
-        self.initial_graphs = graph_generation_params.adapter.adapt(initial_graphs) if initial_graphs else None
+        initial_graphs = graph_generation_params.adapter.adapt(initial_graphs) if initial_graphs else None
+        self.initial_graphs = [graph for graph in initial_graphs if graph_generation_params.verifier(graph)] \
+            if initial_graphs else None
         self.requirements = requirements or OptimizationParameters()
         self.graph_generation_params = graph_generation_params or GraphGenerationParams()
         self.graph_optimizer_params = graph_optimizer_params or AlgorithmParameters()

test/integration/test_evolution_with_crossover.py

@@ -0,0 +1,57 @@
+from functools import partial
+
+import networkx as nx
+import pytest
+
+from examples.synthetic_graph_evolution.generators import generate_labeled_graph
+from golem.core.adapter.nx_adapter import BaseNetworkxAdapter
+from golem.core.dag.verification_rules import DEFAULT_DAG_RULES
+from golem.core.log import Log
+from golem.core.optimisers.genetic.gp_optimizer import EvoGraphOptimizer
+from golem.core.optimisers.genetic.gp_params import GPAlgorithmParameters
+from golem.core.optimisers.genetic.operators.base_mutations import MutationTypesEnum
+from golem.core.optimisers.genetic.operators.crossover import CrossoverTypesEnum
+from golem.core.optimisers.objective import Objective
+from golem.core.optimisers.optimization_parameters import GraphRequirements
+from golem.core.optimisers.optimizer import GraphGenerationParams
+from golem.metrics.graph_metrics import spectral_dist
+
+
+@pytest.mark.parametrize('graph_type', ['tree', 'dag'])
+def test_evolution_with_crossover(graph_type):
+    Log().reset_logging_level(10)
+    target_graph = generate_labeled_graph(graph_type, 50)
+    num_iterations = 100
+    objective = Objective(partial(spectral_dist, target_graph))
+
+    requirements = GraphRequirements(
+        early_stopping_iterations=num_iterations,
+        num_of_generations=num_iterations,
+        n_jobs=-1,
+        history_dir=None
+    )
+    gp_params = GPAlgorithmParameters(
+        pop_size=30,
+        mutation_types=[
+            MutationTypesEnum.single_edge,
+            MutationTypesEnum.single_add,
+            MutationTypesEnum.single_drop,
+            MutationTypesEnum.simple,
+            MutationTypesEnum.single_change
+        ],
+        crossover_types=[CrossoverTypesEnum.subtree, CrossoverTypesEnum.one_point]
+    )
+    graph_gen_params = GraphGenerationParams(
+        adapter=BaseNetworkxAdapter(),
+        rules_for_constraint=DEFAULT_DAG_RULES,
+        available_node_types=['x'],
+    )
+
+    # Generate simple initial population with cyclic graphs
+    initial_graphs = [generate_labeled_graph(graph_type, i) for i in range(4, 20)]
+
+    optimiser = EvoGraphOptimizer(objective, initial_graphs, requirements, graph_gen_params, gp_params)
+    found_graphs = optimiser.optimise(objective)
+    found_graph: nx.DiGraph = graph_gen_params.adapter.restore(found_graphs[0])
+    assert found_graph is not None
+    assert len(found_graph.nodes) > 0

test/unit/adapter/graph_data.py

@@ -54,10 +54,10 @@ def graph_with_custom_parameters(alpha_value):
 
 def networkx_graph_with_parameters(alpha_value):
     graph = nx.DiGraph()
-    graph.add_node('a')
-    graph.add_node('b')
-    graph.add_node('c', alpha=alpha_value)
-    graph.add_edges_from([('a', 'c'), ('b', 'c')])
+    graph.add_node(0, name='a')
+    graph.add_node(1, name='b')
+    graph.add_node(2, name='c', alpha=alpha_value)
+    graph.add_edges_from([(0, 2), (1, 2)])
     return graph
 
 

test/unit/adapter/test_adapter.py

@@ -34,7 +34,8 @@ def test_adapters_params_correct(adapter, graph_with_params):
     if isinstance(graph, Graph):
         restored_alpha = restored_graph.root_node.content['params']['alpha']
     else:
-        restored_alpha = restored_graph.nodes['c']['alpha']
+        root_node = [node for node in restored_graph.nodes() if restored_graph.out_degree(node) == 0][0]
+        restored_alpha = restored_graph.nodes[root_node]['alpha']
     assert np.isclose(init_alpha, restored_alpha)