From 16aafb13ffc896b1fe9f63057de610927d0f83c1 Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Mon, 17 Aug 2026 01:39:08 +0300 Subject: [PATCH 1/2] Order the cycles a graph returns by vertex instead of by hash get_disparate_cycles(), get_polycycles() and get_all_cycles_of_size() all build their cycles as sets of vertices and hand them back as list(cycle_set), so the order of the atoms within a cycle came out of the set's iteration. Atom hashes on ('Atom', symbol) and compares by identity, so every carbon in a molecule lands in one hash bucket and the order of a cycle follows Python's randomised string hash. The same molecule therefore yields differently ordered cycles in different processes. calculate_cyclic_symmetry_number() consumes that order. For a polycyclic cluster it calls get_largest_ring(ring[0]), seeding a largest-ring search from whichever atom the set happened to list first. In a bridged polycycle the largest ring through the bridge atom is smaller than the largest ring through any other atom, so the ring handed to the symmetry search changes size with the hash seed and the symmetry number changes with it: bicyclo[2.2.1]heptane returned 4 for 163 of the hash seeds 0-200 and 2 for the other 38, and 7-oxabicyclo[2.2.1]heptane returned 4 for 128 and 2 for 73. A symmetry number enters the entropy as -R ln(sigma), so a factor of two is 1.377 cal/mol/K in S and a factor of two in every rate and equilibrium constant for the species. kekulize() consumes the same order through get_all_cycles_of_size(6), which seeds its ring-by-ring resolution with the ring list. Return the vertices of every cycle in the graph's vertex order instead, so the order is the same in every process. --- arc/molecule/graph.pxd | 2 ++ arc/molecule/graph.pyx | 41 +++++++++++++++++++++++++++++++++-------- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/arc/molecule/graph.pxd b/arc/molecule/graph.pxd index 8609576e6a..59687f5f79 100644 --- a/arc/molecule/graph.pxd +++ b/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) diff --git a/arc/molecule/graph.pyx b/arc/molecule/graph.pyx index d6382048c9..c347e09252 100644 --- a/arc/molecule/graph.pyx +++ b/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 From bc7d45d3db8828db4f60b189ae22749b01147cd1 Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Mon, 17 Aug 2026 01:39:16 +0300 Subject: [PATCH 2/2] Test that a graph's cycle order is stable across processes Cover both halves of the contract: order_vertex_set() puts a set of vertices back into the graph's vertex order, and two processes started with different hash seeds list the cycles, and the symmetry numbers derived from them, identically. The symmetry test asserts only that the processes agree, not what they agree on. What calculate_cyclic_symmetry_number() should return for a bridged polycycle is a separate question from whether it returns the same thing twice. PYTHONPATH is pinned to the repository so the child process imports the tree under test rather than an installed ARC. --- arc/molecule/graph_test.py | 39 +++++++++++++++++++++++++++++++++++ arc/molecule/symmetry_test.py | 29 ++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/arc/molecule/graph_test.py b/arc/molecule/graph_test.py index 6a8f11b2c1..ef9cd86e2d 100644 --- a/arc/molecule/graph_test.py +++ b/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. diff --git a/arc/molecule/symmetry_test.py b/arc/molecule/symmetry_test.py index cc32f28881..43d02e6d70 100644 --- a/arc/molecule/symmetry_test.py +++ b/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))