diff --git a/rmgpy/data/base.py b/rmgpy/data/base.py index ba94671e982..89fba1f6736 100644 --- a/rmgpy/data/base.py +++ b/rmgpy/data/base.py @@ -1058,18 +1058,14 @@ def descend_tree(self, structure, atoms, root=None, strict=False): if self.match_node_to_structure(child, structure, atoms, strict): next_node.append(child) - if len(next_node) == 1: + if len(next_node) != 0: + # at least one child matches - pick the first in tree order. return self.descend_tree(structure, atoms, next_node[0], strict) - elif len(next_node) == 0: + else: if len(root.children) > 0 and root.children[-1].label.startswith('Others-'): return root.children[-1] else: return root - else: - # logging.warning('For {0}, a node {1} with overlapping children {2} was encountered ' - # 'in tree with top level nodes {3}. Assuming the first match is the ' - # 'better one.'.format(structure, root, next, self.top)) - return self.descend_tree(structure, atoms, next_node[0], strict) def are_siblings(self, node, node_other): """ diff --git a/rmgpy/data/thermo.py b/rmgpy/data/thermo.py index 064045264ff..359be6ac14c 100644 --- a/rmgpy/data/thermo.py +++ b/rmgpy/data/thermo.py @@ -317,16 +317,6 @@ def common_atoms(cycle1, cycle2): return set1.intersection(set2) -def combine_cycles(cycle1, cycle2): - """ - INPUT: two cycles with type: list of atoms - OUTPUT: a combined cycle with type: list of atoms - """ - set1 = set(cycle1) - set2 = set(cycle2) - return list(set1.union(set2)) - - def is_aromatic_ring(submol): """ This method takes a monoring submol (Molecule initialized with a list of atoms containing just diff --git a/rmgpy/molecule/graph.pyx b/rmgpy/molecule/graph.pyx index 9d958f18f31..a0968a05a0f 100644 --- a/rmgpy/molecule/graph.pyx +++ b/rmgpy/molecule/graph.pyx @@ -469,7 +469,10 @@ cdef class Graph(object): cpdef sort_vertices(self, bint save_order=False): """ Sort the vertices in the graph. This can make certain operations, e.g. - the isomorphism functions, much more efficient. + the isomorphism functions, much more efficient. Vertices that have a + ``sorting_key`` attribute (e.g. :class:`Atom`) will use it as a chemical + tiebreaker for vertices with identical connectivity values, ensuring + deterministic ordering. """ cdef Vertex vertex cdef int index @@ -485,7 +488,30 @@ cdef class Graph(object): # If we need to sort then let's also update the connecitivities so # we're sure they are right, since the sorting labels depend on them self.update_connectivity_values() - self.vertices.sort(key=get_vertex_connectivity_value) + + # Build sort keys with optional chemical tiebreaker (sorting_key attribute) + # for when connectivity values are identical. + # Sort (key, index) pairs, then use the resulting index order to reorder. + paired = [] + for index, vertex in enumerate(self.vertices): + conn = get_vertex_connectivity_value(vertex) + sort_key = getattr(vertex, 'sorting_key', None) + paired.append( + ( + # the sorting key is a tuple of keys which are consumed in order. + # we first attempt to sort by conn (always defined). + # in the case of a tie, sort by the contents of the second element: + # - sort_key, which is a tuple of sortable things defined elsewhere, if it is defined + # - empty tuple, if sort_key is not defined + # the latter results in a tie, which we 'break' by just not changing the order (stable sort) + (conn, (sort_key if sort_key is not None else tuple())), + index + ) + ) + + paired.sort() + ordered = [self.vertices[idx] for _, idx in paired] + self.vertices = ordered for index, vertex in enumerate(self.vertices): vertex.sorting_label = index diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index ca405319a67..a8dff2f235a 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -2838,8 +2838,8 @@ def get_polycycles(self): if vertex in cycle: polycyclic_cycle.update(cycle) - # convert each set to a list - continuous_cycles = [list(cycle) for cycle in continuous_cycles] + # convert each set to a list (sorted for deterministic atom ordering) + continuous_cycles = [sorted(cycle) for cycle in continuous_cycles] return continuous_cycles def get_monocycles(self): @@ -2888,9 +2888,9 @@ def get_disparate_cycles(self): # Merge connected cycles 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] + # Convert cycles back to lists (sorted for deterministic atom ordering) + monocyclic_cycles = [sorted(cycle_set) for cycle_set in monocyclic_cycles] + polycyclic_cycles = [sorted(cycle_set) for cycle_set in polycyclic_cycles] return monocyclic_cycles, polycyclic_cycles diff --git a/rmgpy/reaction.py b/rmgpy/reaction.py index 13817852863..a146bd01666 100644 --- a/rmgpy/reaction.py +++ b/rmgpy/reaction.py @@ -615,7 +615,12 @@ def is_isomorphic(self, other, either_direction=True, check_identical=False, che save_order=save_order) # Compare specific_collider to specific_collider - collider_match = (self.specific_collider == other.specific_collider) + if self.specific_collider is None: + collider_match = other.specific_collider is None + elif other.specific_collider is None: + collider_match = False + else: + collider_match = self.specific_collider.is_isomorphic(other.specific_collider) # Return now, if we can if forward_reactants_match and forward_products_match and collider_match: diff --git a/rmgpy/rmg/main.py b/rmgpy/rmg/main.py index eaa4a732eba..c3dbc66508c 100644 --- a/rmgpy/rmg/main.py +++ b/rmgpy/rmg/main.py @@ -1110,7 +1110,13 @@ def execute(self, initialize=True, **kwargs): # These should be Species or Network objects logging.info("") - objects_to_enlarge = list(set(objects_to_enlarge)) + # objects_to_enlarge can contain Species, Network, or tuples (PDepNetwork, Species) + # Sort deterministically: by label for objects with .label, by first element for tuples + def _sort_key(obj): + if isinstance(obj, tuple): + return (1, getattr(obj[0], 'label', str(obj[0]))) + return (0, getattr(obj, 'label', str(obj))) + objects_to_enlarge = sorted(set(objects_to_enlarge), key=_sort_key) # Add objects to enlarge to the core first for objectToEnlarge in objects_to_enlarge: @@ -1927,7 +1933,7 @@ def process_to_species_networks(self, obj): rspcs = self.process_reactions_to_species([k for k in obj if isinstance(k, Reaction)]) spcs = {k for k in obj if isinstance(k, Species)} | rspcs nworks, pspcs = self.process_pdep_networks([k for k in obj if isinstance(k, PDepNetwork)]) - spcs = list(spcs - pspcs) # avoid duplicate species + spcs = sorted(spcs - pspcs, key=lambda s: s.label) # avoid duplicate species, sort deterministically return spcs + nworks else: raise TypeError("improper call, obj input was incorrect") diff --git a/scripts/rmg2to3.py b/scripts/rmg2to3.py index 0abc81c3b44..6b9a7293062 100644 --- a/scripts/rmg2to3.py +++ b/scripts/rmg2to3.py @@ -291,7 +291,6 @@ 'removeThermoData': 'remove_thermo_data', 'averageThermoData': 'average_thermo_data', 'commonAtoms': 'common_atoms', - 'combineCycles': 'combine_cycles', 'isAromaticRing': 'is_aromatic_ring', 'isBicyclic': 'is_bicyclic', 'findAromaticBondsFromSubMolecule': 'find_aromatic_bonds_from_sub_molecule', diff --git a/test/regression/RMS_CSTR_liquid_oxidation/input.py b/test/regression/RMS_CSTR_liquid_oxidation/input.py index 0808cfeecb4..940141902b9 100644 --- a/test/regression/RMS_CSTR_liquid_oxidation/input.py +++ b/test/regression/RMS_CSTR_liquid_oxidation/input.py @@ -53,12 +53,12 @@ toleranceMoveToCore=0.01, toleranceKeepInEdge=0.001, toleranceInterruptSimulation=1e8, - maximumEdgeSpecies=10000, + maximumEdgeSpecies=300, minCoreSizeForPrune=10, minSpeciesExistIterationsForPrune=2, maxNumObjsPerIter=3, filterReactions=True, - maxNumSpecies=35, + maxNumSpecies=15, ) options( diff --git a/test/rmgpy/data/thermoTest.py b/test/rmgpy/data/thermoTest.py index f3a26b21ea1..b09d5629671 100644 --- a/test/rmgpy/data/thermoTest.py +++ b/test/rmgpy/data/thermoTest.py @@ -42,7 +42,6 @@ ThermoCentralDatabaseInterface, convert_ring_to_sub_molecule, bicyclic_decomposition_for_polyring, - combine_cycles, combine_two_rings_into_sub_molecule, find_aromatic_bonds_from_sub_molecule, get_copy_for_one_ring, @@ -2476,17 +2475,6 @@ def test_deterministic_bicyclic_decomposition(self): except AssertionError as e: pytest.skip(f"Skipping because not yet deterministic (#2562): {e}") - def test_combine_cycles(self): - """ - This method tests the combine_cycles method, which simply joins two lists - together without duplication. - """ - main_cycle = Molecule(smiles="C1CCC2CCCCC2C1").atoms - test_cycle1 = main_cycle[0:8] - test_cycle2 = main_cycle[6:] - joined_cycle = combine_cycles(test_cycle1, test_cycle2) - assert set(main_cycle) == set(joined_cycle) - def test_split_bicyclic_into_single_rings1(self): """ Test bicyclic molecule "C1=CCC2C1=C2" can be divided into