Test that a graph's edge and cycle order is stable across processes

The edge order returned by get_all_edges() and the vertex order within the
cycles returned by get_disparate_cycles(), get_polycycles() and
get_all_cycles_of_size() used to follow the iteration order of a set of Edge or
Vertex objects, which is governed by the per-process randomized string hash, so
this asserts the property directly: subprocesses started at different
PYTHONHASHSEED values must report the same order, and the same symmetry numbers.

The symmetry numbers are checked at both entry points. calculate_symmetry_number()
is the one that reads the cycles, and ARCSpecies.get_symmetry_number() is the one
production calls, which reaches the symmetry code through get_resonance_hybrid()
rather than through the molecule it was given, so the resonance layer is covered
too. Both assert only that the processes agree, not what they agree on. What
calculate_cyclic_symmetry_number() should return for a bridged polycycle or a
peri-fused aromatic is a separate question from whether it returns the same thing
twice, and asserting a value here would fix the wrong one in place.

order_vertex_set() is covered for the ordering itself and for its rejection of a
vertex that does not belong to the graph, including the vertices of a copy of the
graph, which compare unequal to the originals and would otherwise be dropped.

The child processes are given PYTHONPATH and a working directory explicitly. A
subprocess inherits the parent's working directory but not pytest's sys.path, so
without it the child imports whichever ARC `import arc` resolves to -- which,
with an editable install present, is not necessarily the tree under test. The
test then either fails spuriously when run from outside the repository root, or
passes while having validated a different checkout.

Author: calvinp0

arc/molecule/graph_test.py

@@ -1,11 +1,38 @@
 #!/usr/bin/env python3
 # encoding: utf-8
 
+import os
+import subprocess
+import sys
 import unittest
 
 from arc.molecule.graph import Edge, Graph, Vertex
 
 
+REPOSITORY_DIRECTORY = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
+
+def outputs_at_hash_seeds(script, seeds):
+    """
+    Run `script` in a subprocess once per hash seed in `seeds`, and return the stripped standard
+    output of each run. The subprocesses are given the repository as their working directory and at
+    the front of PYTHONPATH, so they import the tree under test rather than an installed ARC while
+    keeping whatever else the environment already put on the path.
+
+    Raises a RuntimeError if any of the subprocesses exits with a non-zero return code.
+    """
+    python_path = os.pathsep.join(path for path in (REPOSITORY_DIRECTORY, os.environ.get('PYTHONPATH', '')) if path)
+    outputs = list()
+    for seed in seeds:
+        environment = dict(os.environ, PYTHONHASHSEED=seed, PYTHONPATH=python_path)
+        result = subprocess.run([sys.executable, '-c', script], capture_output=True, text=True,
+                                env=environment, cwd=REPOSITORY_DIRECTORY, timeout=600)
+        if result.returncode:
+            raise RuntimeError(f'The subprocess run with PYTHONHASHSEED={seed} failed:\n{result.stderr}')
+        outputs.append(result.stdout.strip())
+    return outputs
+
+
 class TestGraph(unittest.TestCase):
     """
     Contains unit tests of the Vertex, Edge, and Graph classes. Most of the
@@ -108,6 +135,86 @@ def test_get_all_edges(self):
         self.assertIsInstance(edges, list)
         self.assertEqual(len(edges), 5)
 
+    def test_get_all_edges_orders_the_edges_by_vertex(self):
+        """
+        Test that Graph.get_all_edges() returns the edges in vertex order, each edge once.
+        """
+        expected = []
+        for vertex in self.graph.vertices:
+            for edge in vertex.edges.values():
+                if not any(edge is seen for seen in expected):
+                    expected.append(edge)
+        self.assertEqual(self.graph.get_all_edges(), expected)
+
+    def test_get_all_edges_order_does_not_depend_on_the_hash_seed(self):
+        """
+        Test that Graph.get_all_edges() returns the same order in processes with different hash seeds.
+        """
+        script = ('from arc.molecule.molecule import Molecule\n'
+                  'mol = Molecule(smiles="c1ccccc1Cc1ccccc1")\n'
+                  'atoms = mol.atoms\n'
+                  'print([(atoms.index(edge.vertex1), atoms.index(edge.vertex2)) '
+                  'for edge in mol.get_all_edges()])\n')
+        outputs = outputs_at_hash_seeds(script, ('1', '35'))
+        self.assertTrue(outputs[0])
+        self.assertEqual(len(set(outputs)), 1, f'The edge order differs between hash seeds: {outputs}')
+
+    def test_order_vertex_set(self):
+        """
+        Test that Graph.order_vertex_set() returns the vertices in the graph's vertex order.
+        """
+        vertices = self.graph.vertices
+        self.assertEqual(self.graph.order_vertex_set({vertices[4], vertices[1], vertices[3]}),
+                         [vertices[1], vertices[3], vertices[4]])
+        self.assertEqual(self.graph.order_vertex_set(set()), [])
+        self.assertEqual(self.graph.order_vertex_set(set(vertices)), vertices)
+
+    def test_order_vertex_set_rejects_a_vertex_that_is_not_in_the_graph(self):
+        """
+        Test that Graph.order_vertex_set() raises a ValueError instead of dropping a foreign vertex.
+        """
+        vertices = self.graph.vertices
+        with self.assertRaises(ValueError):
+            self.graph.order_vertex_set({Vertex()})
+        with self.assertRaises(ValueError):
+            self.graph.order_vertex_set({vertices[0], vertices[2], Vertex()})
+        copied = self.graph.copy(deep=True)
+        with self.assertRaises(ValueError):
+            self.graph.order_vertex_set(set(copied.vertices))
+
+    def test_order_vertex_set_counts_a_repeated_vertex_once(self):
+        """
+        Test that Graph.order_vertex_set() returns a vertex once and still rejects a foreign vertex
+        when the graph's vertex list holds the same vertex twice.
+        """
+        vertices = self.graph.vertices
+        repeated = Graph(vertices=[vertices[0], vertices[0], vertices[1]])
+        self.assertEqual(repeated.order_vertex_set({vertices[0], vertices[1]}),
+                         [vertices[0], vertices[1]])
+        with self.assertRaises(ValueError):
+            repeated.order_vertex_set({vertices[0], Vertex()})
+
+    def test_cycle_order_does_not_depend_on_the_hash_seed(self):
+        """
+        Test that the cycles a graph returns are ordered identically in processes with different hash seeds.
+
+        The comparison covers the order of the vertices within one cycle and the order of the cycles
+        within the returned list.
+        """
+        script = ('from arc.molecule.molecule import Molecule\n'
+                  'for smiles in ("C1CC2CCC1C2", "c1ccccc1Cc1ccccc1", "C1CC2CCC3CCC1C23"):\n'
+                  '    mol = Molecule(smiles=smiles)\n'
+                  '    atoms = mol.atoms\n'
+                  '    index = lambda cycle: [atoms.index(atom) for atom in cycle]\n'
+                  '    monocyclic, polycyclic = mol.get_disparate_cycles()\n'
+                  '    print(smiles, [index(cycle) for cycle in monocyclic + polycyclic])\n'
+                  '    print(smiles, [index(cycle) for cycle in mol.get_polycycles()])\n'
+                  '    print(smiles, [index(cycle) for cycle in mol.get_all_cycles_of_size(5)])\n'
+                  '    print(smiles, [index(cycle) for cycle in mol.get_smallest_set_of_smallest_rings()])\n')
+        outputs = outputs_at_hash_seeds(script, ('1', '5', '87'))
+        self.assertTrue(outputs[0])
+        self.assertEqual(len(set(outputs)), 1, f'The cycle order differs between hash seeds: {outputs}')
+
     def test_has_vertex(self):
         """
         Test the Graph.has_vertex() method.

arc/molecule/symmetry_test.py

@@ -3,13 +3,28 @@
 
 import unittest
 
+from arc.molecule.graph_test import outputs_at_hash_seeds
 from arc.molecule.molecule import Molecule
 from arc.molecule.resonance import generate_optimal_aromatic_resonance_structures
 from arc.molecule.symmetry import (calculate_atom_symmetry_number, calculate_axis_symmetry_number,
     calculate_bond_symmetry_number, calculate_cyclic_symmetry_number, _indistinguishable)
 from arc.species.species import ARCSpecies
 
 
+HASH_SEED_SPECIES = ('C1CC2CCC1C2',
+                     'C1CC2CCC1CC2',
+                     'C1CC2CCC3CCC1C23',
+                     'C1CC2CCC1O2',
+                     'C1=CC2C=CC1C2',
+                     'c1cc2ccc3cccc4ccc(c1)c2c34',
+                     'c1ccc2c(c1)-c1cccc3cccc2c13',
+                     'c1cc2cccc3c4cccc5cccc(c(c1)c23)c54',
+                     'c1cc2ccc3ccc4ccc5ccc6ccc1c1c2c3c4c5c61',
+                     '[CH2]c1ccc2ccccc2c1')
+
+HASH_SEEDS = ('1', '3', '5', '13')
+
+
 class TestMoleculeSymmetry(unittest.TestCase):
     """
     Contains unit tests of the methods for computing symmetry numbers for a
@@ -676,6 +691,39 @@ def test_indistinguishable_2(self):
         # O is different from H
         self.assertFalse(_indistinguishable(mol.atoms[6], mol.atoms[7]))
 
+    def test_symmetry_number_does_not_depend_on_the_hash_seed(self):
+        """
+        Test that calculate_symmetry_number() returns the same value in processes with different hash seeds.
+
+        The bridged polycyclics and fused aromatics of HASH_SEED_SPECIES reach
+        calculate_cyclic_symmetry_number() through get_disparate_cycles(). Only agreement between the
+        processes is asserted, not the value they agree on.
+        """
+        script = ('from arc.molecule.molecule import Molecule\n'
+                  'from arc.molecule.symmetry import calculate_symmetry_number\n'
+                  f'print([calculate_symmetry_number(Molecule(smiles=smiles)) for smiles in {HASH_SEED_SPECIES}])\n')
+        symmetry_numbers = outputs_at_hash_seeds(script, HASH_SEEDS)
+        self.assertTrue(symmetry_numbers[0])
+        self.assertEqual(len(set(symmetry_numbers)), 1,
+                         f'The symmetry numbers differ between hash seeds: {symmetry_numbers}')
+
+    def test_species_symmetry_number_does_not_depend_on_the_hash_seed(self):
+        """
+        Test that ARCSpecies.get_symmetry_number() returns the same value in processes with different hash seeds.
+
+        This is the entry point production uses. It reaches the symmetry code through
+        get_resonance_hybrid() rather than through the molecule it was given, so the resonance layer
+        lies between the caller and the cycles. Only agreement between the processes is asserted, not
+        the value they agree on.
+        """
+        script = ('from arc.species.species import ARCSpecies\n'
+                  'print([ARCSpecies(label=f"species{index}", smiles=smiles).get_symmetry_number() '
+                  f'for index, smiles in enumerate({HASH_SEED_SPECIES})])\n')
+        symmetry_numbers = outputs_at_hash_seeds(script, HASH_SEEDS)
+        self.assertTrue(symmetry_numbers[0])
+        self.assertEqual(len(set(symmetry_numbers)), 1,
+                         f'The symmetry numbers differ between hash seeds: {symmetry_numbers}')
+
 
 if __name__ == '__main__':
     unittest.main(testRunner=unittest.TextTestRunner(verbosity=2))