Make cycle order, and therefore symmetry numbers, independent of PYTHONHASHSEED

arc/molecule/graph.pxd

@@ -54,6 +54,8 @@ cdef class Graph(object):
 
     cpdef list get_all_edges(self)
 
+    cpdef list order_vertex_set(self, set vertex_set)
+
     cpdef dict get_edges(self, Vertex vertex)
 
     cpdef Edge get_edge(self, Vertex vertex1, Vertex vertex2)

arc/molecule/graph.pyx

@@ -231,6 +231,22 @@ cdef class Graph(object):
 
         return list(edge_set)
 
+    cpdef list order_vertex_set(self, set vertex_set):
+        """
+        Returns the vertices of `vertex_set` as a list, ordered by their position
+        in the graph's vertex list. The order does not depend on the hash values
+        of the vertices, so it is identical in every process.
+        """
+        cdef list ordered
+        cdef Vertex vertex
+
+        ordered = []
+        for vertex in self.vertices:
+            if vertex in vertex_set:
+                ordered.append(vertex)
+
+        return ordered
+
     cpdef dict get_edges(self, Vertex vertex):
         """
         Return a dictionary of the edges involving the specified `vertex`.
@@ -615,9 +631,12 @@ cdef class Graph(object):
     cpdef list get_polycycles(self):
         """
         Return a list of cycles that are polycyclic.
-        In other words, merge the cycles which are fused or spirocyclic into 
-        a single polycyclic cycle, and return only those cycles. 
+        In other words, merge the cycles which are fused or spirocyclic into
+        a single polycyclic cycle, and return only those cycles.
         Cycles which are not polycyclic are not returned.
+
+        The vertices of each returned cycle are ordered by their position in the
+        graph's vertex list, so the order is identical in every process.
         """
         cdef list polycyclic_vertices, continuous_cycles, sssr
         cdef set polycyclic_cycle
@@ -652,7 +671,7 @@ cdef class Graph(object):
                         polycyclic_cycle.update(cycle)
 
             # convert each set to a list
-            continuous_cycles = [list(cycle) for cycle in continuous_cycles]
+            continuous_cycles = [self.order_vertex_set(cycle) for cycle in continuous_cycles]
             return continuous_cycles
 
     cpdef list get_monocycles(self):
@@ -690,7 +709,10 @@ cdef class Graph(object):
         """
         Get all disjoint monocyclic and polycyclic cycle clusters in the molecule.
         Takes the RC and recursively merges all cycles which share vertices.
-
+
+        The vertices of each returned cycle are ordered by their position in the
+        graph's vertex list, so the order is identical in every process.
+
         Returns: monocyclic_cycles, polycyclic_cycles
         """
         cdef list rc, cycle_list, cycle_sets, monocyclic_cycles, polycyclic_cycles
@@ -708,8 +730,8 @@ cdef class Graph(object):
         monocyclic_cycles, polycyclic_cycles = self._merge_cycles(cycle_sets)
 
         # Convert cycles back to lists
-        monocyclic_cycles = [list(cycle_set) for cycle_set in monocyclic_cycles]
-        polycyclic_cycles = [list(cycle_set) for cycle_set in polycyclic_cycles]
+        monocyclic_cycles = [self.order_vertex_set(cycle_set) for cycle_set in monocyclic_cycles]
+        polycyclic_cycles = [self.order_vertex_set(cycle_set) for cycle_set in polycyclic_cycles]
 
         return monocyclic_cycles, polycyclic_cycles
 
@@ -779,7 +801,10 @@ cdef class Graph(object):
     cpdef list get_all_cycles_of_size(self, int size):
         """
         Return a list of the all non-duplicate rings with length 'size'. The
-        algorithm implements was adapted from a description by Fan, Panaye,
+        vertices of each ring are ordered by their position in the graph's vertex
+        list, so the order is identical in every process.
+
+        The algorithm implements was adapted from a description by Fan, Panaye,
         Doucet, and Barbu (doi: 10.1021/ci00015a002)
 
         B. T. Fan, A. Panaye, J. P. Doucet, and A. Barbu. "Ring Perception: A
@@ -884,7 +909,7 @@ cdef class Graph(object):
                 cycle_set_list.append(set1)
 
         #transform back to list of lists:
-        cycle_set_list = [list(set1) for set1 in cycle_set_list]
+        cycle_set_list = [self.order_vertex_set(set1) for set1 in cycle_set_list]
 
         return cycle_set_list
 

arc/molecule/graph_test.py

@@ -1,11 +1,17 @@
 #!/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__))))
+
+
 class TestGraph(unittest.TestCase):
     """
     Contains unit tests of the Vertex, Edge, and Graph classes. Most of the
@@ -108,6 +114,39 @@ def test_get_all_edges(self):
         self.assertIsInstance(edges, list)
         self.assertEqual(len(edges), 5)
 
+    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_cycle_vertex_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.
+        """
+        script = ('from arc.molecule.molecule import Molecule\n'
+                  'mol = Molecule(smiles="C1CC2CCC1C2")\n'
+                  'atoms = mol.atoms\n'
+                  'index = lambda cycle: [atoms.index(atom) for atom in cycle]\n'
+                  'monocyclic, polycyclic = mol.get_disparate_cycles()\n'
+                  'print([index(cycle) for cycle in monocyclic + polycyclic])\n'
+                  'print([index(cycle) for cycle in mol.get_polycycles()])\n'
+                  'print([index(cycle) for cycle in mol.get_all_cycles_of_size(5)])\n')
+        orders = list()
+        for seed in ('1', '5', '87'):
+            environment = dict(os.environ, PYTHONHASHSEED=seed, PYTHONPATH=REPOSITORY_DIRECTORY)
+            result = subprocess.run([sys.executable, '-c', script], capture_output=True, text=True,
+                                    env=environment, cwd=REPOSITORY_DIRECTORY, timeout=300)
+            self.assertEqual(result.returncode, 0, f'Subprocess failed: {result.stderr}')
+            orders.append(result.stdout.strip())
+        self.assertTrue(orders[0])
+        for order in orders[1:]:
+            self.assertEqual(orders[0], order)
+
     def test_has_vertex(self):
         """
         Test the Graph.has_vertex() method.

arc/molecule/symmetry_test.py

@@ -1,6 +1,9 @@
 #!/usr/bin/env python3
 # encoding: utf-8
 
+import os
+import subprocess
+import sys
 import unittest
 
 from arc.molecule.molecule import Molecule
@@ -10,6 +13,9 @@
 from arc.species.species import ARCSpecies
 
 
+REPOSITORY_DIRECTORY = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
+
 class TestMoleculeSymmetry(unittest.TestCase):
     """
     Contains unit tests of the methods for computing symmetry numbers for a
@@ -676,6 +682,29 @@ 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 listed here reach calculate_cyclic_symmetry_number() through
+        get_disparate_cycles(), whose cycles used to be ordered by the hash values of their atoms.
+        The value asserted is only that every process agrees, not what that value is.
+        """
+        script = ('from arc.molecule.molecule import Molecule\n'
+                  'from arc.molecule.symmetry import calculate_symmetry_number\n'
+                  'print([calculate_symmetry_number(Molecule(smiles=smiles)) '
+                  'for smiles in ("C1CC2CCC1C2", "C1CC2CCC1CC2", "C1CC2CCC3CCC1C23", "C1CC2CCC1O2")])\n')
+        symmetry_numbers = list()
+        for seed in ('1', '5', '87'):
+            environment = dict(os.environ, PYTHONHASHSEED=seed, PYTHONPATH=REPOSITORY_DIRECTORY)
+            result = subprocess.run([sys.executable, '-c', script], capture_output=True, text=True,
+                                    env=environment, cwd=REPOSITORY_DIRECTORY, timeout=300)
+            self.assertEqual(result.returncode, 0, f'Subprocess failed: {result.stderr}')
+            symmetry_numbers.append(result.stdout.strip())
+        self.assertTrue(symmetry_numbers[0])
+        for symmetry_number in symmetry_numbers[1:]:
+            self.assertEqual(symmetry_numbers[0], symmetry_number)
+
 
 if __name__ == '__main__':
     unittest.main(testRunner=unittest.TextTestRunner(verbosity=2))