From 3da3c0e346b90faea00704075cfdd130d0c8901b Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 25 Jan 2018 17:14:38 -0500 Subject: [PATCH 01/20] Refactor some class properties using @property decorator More pythonic and prevents extra getter/setter functions from cluttering the namespace. --- rmgpy/molecule/group.py | 15 ++++++++++++--- rmgpy/molecule/molecule.py | 36 +++++++++++++++++++++++++----------- rmgpy/reaction.py | 14 +++++++++++--- rmgpy/species.py | 9 ++++++--- 4 files changed, 54 insertions(+), 20 deletions(-) diff --git a/rmgpy/molecule/group.py b/rmgpy/molecule/group.py index 74b8064a50..8bc19c00da 100644 --- a/rmgpy/molecule/group.py +++ b/rmgpy/molecule/group.py @@ -1010,9 +1010,18 @@ def draw(self, format): img = graph.create(prog='neato', format=format) return img - def __getAtoms(self): return self.vertices - def __setAtoms(self, atoms): self.vertices = atoms - atoms = property(__getAtoms, __setAtoms) + @property + def atoms(self): + """ + List of atoms contained in the current molecule. + + Renames the inherited vertices attribute of :class:`Graph`. + """ + return self.vertices + + @atoms.setter + def atoms(self, atoms): + self.vertices = atoms def addAtom(self, atom): """ diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index 82065da397..eed95c72cf 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -857,23 +857,37 @@ def __reduce__(self): """ return (Molecule, (self.vertices, self.symmetryNumber, self.multiplicity, self.reactive, self.props)) - def __getAtoms(self): return self.vertices - def __setAtoms(self, atoms): self.vertices = atoms - atoms = property(__getAtoms, __setAtoms) + @property + def atoms(self): + """ + List of atoms contained in the current molecule. + + Renames the inherited vertices attribute of :class:`Graph`. + """ + return self.vertices - def __getFingerprint(self): + @atoms.setter + def atoms(self, atoms): + self.vertices = atoms + + @property + def fingerprint(self): """ - Return a string containing the "fingerprint" used to accelerate graph - isomorphism comparisons with other molecules. The fingerprint is a - short string containing a summary of selected information about the - molecule. Two fingerprint strings matching is a necessary (but not - sufficient) condition for the associated molecules to be isomorphic. + Fingerprint used to accelerate graph isomorphism comparisons with + other molecules. The fingerprint is a short string containing a + summary of selected information about the molecule. Two fingerprint + strings matching is a necessary (but not sufficient) condition for + the associated molecules to be isomorphic. + + Currently, the fingerprint is simply the chemical formula. """ if self._fingerprint is None: self.fingerprint = self.getFormula() return self._fingerprint - def __setFingerprint(self, fingerprint): self._fingerprint = fingerprint - fingerprint = property(__getFingerprint, __setFingerprint) + + @fingerprint.setter + def fingerprint(self, fingerprint): + self._fingerprint = fingerprint def addAtom(self, atom): """ diff --git a/rmgpy/reaction.py b/rmgpy/reaction.py index 499e586629..0f4e0ab7c9 100644 --- a/rmgpy/reaction.py +++ b/rmgpy/reaction.py @@ -200,10 +200,19 @@ def __reduce__(self): self.comment )) - def __getDegneneracy(self): + @property + def degeneracy(self): + """ + The reaction path degeneracy for this reaction. + + If the reaction has kinetics, changing the degeneracy + will adjust the reaction rate by a ratio of the new + degeneracy to the old degeneracy. + """ return self._degeneracy - def __setDegeneracy(self, new): + @degeneracy.setter + def degeneracy(self, new): # modify rate if kinetics exists if self.kinetics is not None: if self._degeneracy < 2: @@ -220,7 +229,6 @@ def __setDegeneracy(self, new): self.kinetics.changeRate(degeneracyRatio) # set new degeneracy self._degeneracy = new - degeneracy = property(__getDegneneracy, __setDegeneracy) def toChemkin(self, speciesList=None, kinetics=True): """ diff --git a/rmgpy/species.py b/rmgpy/species.py index 3e7388b4b6..9052985799 100644 --- a/rmgpy/species.py +++ b/rmgpy/species.py @@ -781,11 +781,14 @@ def __reduce__(self): """ return (TransitionState, (self.label, self.conformer, self.frequency, self.tunneling, self.degeneracy)) - def getFrequency(self): + @property + def frequency(self): + """The negative frequency of the first-order saddle point.""" return self._frequency - def setFrequency(self, value): + + @frequency.setter + def frequency(self, value): self._frequency = quantity.Frequency(value) - frequency = property(getFrequency, setFrequency, """The negative frequency of the first-order saddle point.""") def getPartitionFunction(self, T): """ From db1e492d3ea8d6587d187f67420b1989602ecba7 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 25 Jan 2018 18:31:07 -0500 Subject: [PATCH 02/20] Refactor Molecule SMILES and InChI attributes Allow instantiation using either SMILES or InChI Change SMILES and InChI to all lowercase for easier typing Make smiles and inchi read only properties that generate and save the respective identifiers to private attributes --- rmgpy/molecule/molecule.pxd | 7 +++--- rmgpy/molecule/molecule.py | 43 ++++++++++++++++++++++++++++-------- rmgpy/molecule/translator.py | 2 -- 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/rmgpy/molecule/molecule.pxd b/rmgpy/molecule/molecule.pxd index d0d11b256e..5696a2661e 100644 --- a/rmgpy/molecule/molecule.pxd +++ b/rmgpy/molecule/molecule.pxd @@ -139,10 +139,11 @@ cdef class Molecule(Graph): cdef public bint reactive cdef public object rdMol cdef public int rdMolConfId - cdef str _fingerprint - cdef public str InChI cdef public dict props - + cdef str _fingerprint + cdef str _inchi + cdef str _smiles + cpdef addAtom(self, Atom atom) cpdef addBond(self, Bond bond) diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index eed95c72cf..c9301c9b90 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -780,6 +780,7 @@ class Molecule(Graph): ======================= =========== ======================================== Attribute Type Description ======================= =========== ======================================== + `atoms` ``list`` A list of Atom objects in the molecule `symmetryNumber` ``float`` The (estimated) external + internal symmetry number of the molecule, modified for chirality `multiplicity` ``int`` The multiplicity of this species, multiplicity = 2*total_spin+1 `reactive` ``bool`` ``True`` (by default) if the molecule participates in reaction families. @@ -787,7 +788,7 @@ class Molecule(Graph): representative resonance structure was generated by a template reaction `props` ``dict`` A list of properties describing the state of the molecule. `InChI` ``str`` A string representation of the molecule in InChI - `atoms` ``list`` A list of Atom objects in the molecule + `SMILES` ``str`` A string representation of the molecule in SMILES `fingerprint` ``str`` A representation for fast comparison, set as molecular formula ======================= =========== ======================================== @@ -795,18 +796,28 @@ class Molecule(Graph): `InChI` string representing the molecular structure. """ - def __init__(self, atoms=None, symmetry=-1, multiplicity=-187, reactive=True, props=None, SMILES=''): + def __init__(self, atoms=None, symmetry=-1, multiplicity=-187, reactive=True, props=None, InChI='', SMILES=''): Graph.__init__(self, atoms) self.symmetryNumber = symmetry self.multiplicity = multiplicity self.reactive = reactive self._fingerprint = None - self.InChI = '' - if SMILES != '': self.fromSMILES(SMILES) + self._inchi = None + self._smiles = None self.props = props or {} + + if InChI and SMILES: + logging.warning('Both InChI and SMILES provided for Molecule instantiation, using InChI and ignoring SMILES.') + if InChI: + self.fromInChI(InChI) + self._inchi = InChI + elif SMILES: + self.fromSMILES(SMILES) + self._smiles = SMILES + if multiplicity != -187: # it was set explicitly, so re-set it (fromSMILES etc may have changed it) self.multiplicity = multiplicity - + def __deepcopy__(self, memo): return self.copy(deep=True) @@ -889,11 +900,25 @@ def fingerprint(self): def fingerprint(self, fingerprint): self._fingerprint = fingerprint + @property + def InChI(self): + """InChI string for this molecule. Read-only.""" + if self._inchi is None: + self._inchi = self.toInChI() + return self._inchi + + @property + def SMILES(self): + """SMILES string for this molecule. Read-only.""" + if self._smiles is None: + self._smiles = self.toSMILES() + return self._smiles + def addAtom(self, atom): """ Add an `atom` to the graph. The atom is initialized with no bonds. """ - self._fingerprint = None + self._fingerprint = self._inchi = self._smiles = None return self.addVertex(atom) def addBond(self, bond): @@ -901,7 +926,7 @@ def addBond(self, bond): Add a `bond` to the graph as an edge connecting the two atoms `atom1` and `atom2`. """ - self._fingerprint = None + self._fingerprint = self._inchi = self._smiles = None return self.addEdge(bond) def getBonds(self, atom): @@ -950,7 +975,7 @@ def removeAtom(self, atom): not remove atoms that no longer have any bonds as a result of this removal. """ - self._fingerprint = None + self._fingerprint = self._inchi = self._smiles = None return self.removeVertex(atom) def removeBond(self, bond): @@ -959,7 +984,7 @@ def removeBond(self, bond): Does not remove atoms that no longer have any bonds as a result of this removal. """ - self._fingerprint = None + self._fingerprint = self._inchi = self._smiles = None return self.removeEdge(bond) def removeVanDerWaalsBonds(self): diff --git a/rmgpy/molecule/translator.py b/rmgpy/molecule/translator.py index 026a6df834..d412818c6b 100644 --- a/rmgpy/molecule/translator.py +++ b/rmgpy/molecule/translator.py @@ -260,8 +260,6 @@ def fromInChI(mol, inchistr, backend='try-all'): a user-specified backend for conversion, currently supporting rdkit (default) and openbabel. """ - mol.InChI = inchistr - if inchiutil.INCHI_PREFIX in inchistr: return _read(mol, inchistr, 'inchi', backend) else: From aeaf3b5d70ee890e84029fbe4688daace22c2de1 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 25 Jan 2018 18:37:36 -0500 Subject: [PATCH 03/20] Add read-only inchi property to Species --- rmgpy/species.pxd | 1 + rmgpy/species.py | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/rmgpy/species.pxd b/rmgpy/species.pxd index 71d84d6468..10cedfa576 100644 --- a/rmgpy/species.pxd +++ b/rmgpy/species.pxd @@ -53,6 +53,7 @@ cdef class Species: cdef public bint isSolvent cdef public int creationIteration cdef public bint explicitlyAllowed + cdef str _inchi cpdef generate_resonance_structures(self, bint keep_isomorphic=?, bint filter_structures=?) diff --git a/rmgpy/species.py b/rmgpy/species.py index 9052985799..9380565be7 100644 --- a/rmgpy/species.py +++ b/rmgpy/species.py @@ -106,6 +106,7 @@ def __init__(self, index=-1, label='', thermo=None, conformer=None, self.isSolvent = False self.creationIteration = creationIteration self.explicitlyAllowed = explicitlyAllowed + self._inchi = None # Check multiplicity of each molecule is the same if molecule is not None and len(molecule)>1: mult = molecule[0].multiplicity @@ -154,6 +155,14 @@ def __reduce__(self): """ return (Species, (self.index, self.label, self.thermo, self.conformer, self.molecule, self.transportData, self.molecularWeight, self.energyTransferModel, self.reactive, self.props)) + @property + def InChI(self): + """InChI string representation of this species. Read-only.""" + if self._inchi is None: + if self.molecule: + self._inchi = self.molecule[0].InChI + return self._inchi + @property def molecularWeight(self): """The molecular weight of the species. (Note: value_si is in kg/molecule not kg/mol)""" From 3594fc5cc968b1140ab1716330b302e5c767252d Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 25 Jan 2018 18:48:42 -0500 Subject: [PATCH 04/20] Add read-only fingerprint property to Species --- rmgpy/species.pxd | 1 + rmgpy/species.py | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/rmgpy/species.pxd b/rmgpy/species.pxd index 10cedfa576..c24f2abb92 100644 --- a/rmgpy/species.pxd +++ b/rmgpy/species.pxd @@ -53,6 +53,7 @@ cdef class Species: cdef public bint isSolvent cdef public int creationIteration cdef public bint explicitlyAllowed + cdef str _fingerprint cdef str _inchi cpdef generate_resonance_structures(self, bint keep_isomorphic=?, bint filter_structures=?) diff --git a/rmgpy/species.py b/rmgpy/species.py index 9380565be7..e5ab90e2f9 100644 --- a/rmgpy/species.py +++ b/rmgpy/species.py @@ -106,6 +106,7 @@ def __init__(self, index=-1, label='', thermo=None, conformer=None, self.isSolvent = False self.creationIteration = creationIteration self.explicitlyAllowed = explicitlyAllowed + self._fingerprint = None self._inchi = None # Check multiplicity of each molecule is the same if molecule is not None and len(molecule)>1: @@ -155,6 +156,14 @@ def __reduce__(self): """ return (Species, (self.index, self.label, self.thermo, self.conformer, self.molecule, self.transportData, self.molecularWeight, self.energyTransferModel, self.reactive, self.props)) + @property + def fingerprint(self): + """Fingerprint of this species, taken from molecule attribute. Read-only.""" + if self._fingerprint is None: + if self.molecule: + self._fingerprint = self.molecule[0].fingerprint + return self._fingerprint + @property def InChI(self): """InChI string representation of this species. Read-only.""" From 010513e71adaa4c6ea14572382bcdff6ff5b1941 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Mon, 5 Feb 2018 16:53:11 -0500 Subject: [PATCH 05/20] Add read-only multiplicity property to Species --- rmgpy/species.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rmgpy/species.py b/rmgpy/species.py index e5ab90e2f9..0dcf1c528b 100644 --- a/rmgpy/species.py +++ b/rmgpy/species.py @@ -172,6 +172,14 @@ def InChI(self): self._inchi = self.molecule[0].InChI return self._inchi + @property + def multiplicity(self): + """Fingerprint of this species, taken from molecule attribute. Read-only.""" + if self.molecule: + return self.molecule[0].multiplicity + else: + return None + @property def molecularWeight(self): """The molecular weight of the species. (Note: value_si is in kg/molecule not kg/mol)""" From 1b9884bc73df055872f96b1255e0989471b41cee Mon Sep 17 00:00:00 2001 From: Max Liu Date: Fri, 26 Jan 2018 16:55:26 -0500 Subject: [PATCH 06/20] Add unit tests for new Species properties --- rmgpy/speciesTest.py | 60 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/rmgpy/speciesTest.py b/rmgpy/speciesTest.py index 0ffba17ab6..e55f64e45f 100644 --- a/rmgpy/speciesTest.py +++ b/rmgpy/speciesTest.py @@ -322,6 +322,66 @@ def testGetTransportData(self): self.assertTrue(spc.getTransportData() is spc.transportData) + def test_fingerprint_property(self): + """Test that the fingerprint property works""" + spc = Species().fromAdjacencyList( + """ + 1 C u0 p0 c0 {2,D} {6,S} {7,S} + 2 C u0 p0 c0 {1,D} {3,S} {8,S} + 3 C u0 p0 c0 {2,S} {4,D} {9,S} + 4 C u0 p0 c0 {3,D} {5,S} {10,S} + 5 C u0 p0 c0 {4,S} {6,D} {11,S} + 6 C u0 p0 c0 {1,S} {5,D} {12,S} + 7 H u0 p0 c0 {1,S} + 8 H u0 p0 c0 {2,S} + 9 H u0 p0 c0 {3,S} + 10 H u0 p0 c0 {4,S} + 11 H u0 p0 c0 {5,S} + 12 H u0 p0 c0 {6,S} + """) + + self.assertEqual(spc.fingerprint, 'C6H6') + + def test_inchi_property(self): + """Test that the InChI property works""" + spc = Species().fromAdjacencyList( + """ + 1 C u0 p0 c0 {2,D} {6,S} {7,S} + 2 C u0 p0 c0 {1,D} {3,S} {8,S} + 3 C u0 p0 c0 {2,S} {4,D} {9,S} + 4 C u0 p0 c0 {3,D} {5,S} {10,S} + 5 C u0 p0 c0 {4,S} {6,D} {11,S} + 6 C u0 p0 c0 {1,S} {5,D} {12,S} + 7 H u0 p0 c0 {1,S} + 8 H u0 p0 c0 {2,S} + 9 H u0 p0 c0 {3,S} + 10 H u0 p0 c0 {4,S} + 11 H u0 p0 c0 {5,S} + 12 H u0 p0 c0 {6,S} + """) + + self.assertEqual(spc.InChI, 'InChI=1S/C6H6/c1-2-4-6-5-3-1/h1-6H') + + def test_multiplicity_property(self): + """Test that the fingerprint property works""" + spc = Species().fromAdjacencyList( + """ + 1 C u0 p0 c0 {2,D} {6,S} {7,S} + 2 C u0 p0 c0 {1,D} {3,S} {8,S} + 3 C u0 p0 c0 {2,S} {4,D} {9,S} + 4 C u0 p0 c0 {3,D} {5,S} {10,S} + 5 C u0 p0 c0 {4,S} {6,D} {11,S} + 6 C u0 p0 c0 {1,S} {5,D} {12,S} + 7 H u0 p0 c0 {1,S} + 8 H u0 p0 c0 {2,S} + 9 H u0 p0 c0 {3,S} + 10 H u0 p0 c0 {4,S} + 11 H u0 p0 c0 {5,S} + 12 H u0 p0 c0 {6,S} + """) + + self.assertEqual(spc.multiplicity, 1) + ################################################################################ if __name__ == '__main__': From 3d3473fad2198cba172d80a3cc43831ebb4b07ff Mon Sep 17 00:00:00 2001 From: Max Liu Date: Mon, 27 Aug 2018 13:43:59 -0400 Subject: [PATCH 07/20] Add strict argument for isomorphism comparison For strict=False, ignores electrons (i.e. bond order, radicals, lone pairs, etc.) Only implemented for Molecule isomorphism, exceptions raised in other cases For atoms, strict argument is added to the `equivalent` method For bonds, strict argument is handled within vf2 --- rmgpy/molecule/graph.pxd | 6 ++--- rmgpy/molecule/graph.pyx | 20 ++++++++++++---- rmgpy/molecule/group.pxd | 6 ++--- rmgpy/molecule/group.py | 12 +++++++--- rmgpy/molecule/molecule.pxd | 6 ++--- rmgpy/molecule/molecule.py | 42 +++++++++++++++++++++++----------- rmgpy/molecule/moleculeTest.py | 24 +++++++++++++++++++ rmgpy/molecule/vf2.pxd | 7 +++--- rmgpy/molecule/vf2.pyx | 28 +++++++++++++---------- 9 files changed, 106 insertions(+), 45 deletions(-) diff --git a/rmgpy/molecule/graph.pxd b/rmgpy/molecule/graph.pxd index fcd6ca523e..26cd2af6f6 100644 --- a/rmgpy/molecule/graph.pxd +++ b/rmgpy/molecule/graph.pxd @@ -40,7 +40,7 @@ cdef class Vertex(object): cpdef Vertex copy(self) - cpdef bint equivalent(self, Vertex other) except -2 + cpdef bint equivalent(self, Vertex other, bint strict=?) except -2 cpdef bint isSpecificCaseOf(self, Vertex other) except -2 @@ -110,9 +110,9 @@ cdef class Graph(object): cpdef restore_vertex_order(self) - cpdef bint isIsomorphic(self, Graph other, dict initialMap=?, bint saveOrder=?) except -2 + cpdef bint isIsomorphic(self, Graph other, dict initialMap=?, bint saveOrder=?, bint strict=?) except -2 - cpdef list findIsomorphism(self, Graph other, dict initialMap=?, bint saveOrder=?) + cpdef list findIsomorphism(self, Graph other, dict initialMap=?, bint saveOrder=?, bint strict=?) cpdef bint isSubgraphIsomorphic(self, Graph other, dict initialMap=?, bint saveOrder=?) except -2 diff --git a/rmgpy/molecule/graph.pyx b/rmgpy/molecule/graph.pyx index 7332b6a9db..94c554b5aa 100644 --- a/rmgpy/molecule/graph.pyx +++ b/rmgpy/molecule/graph.pyx @@ -95,7 +95,7 @@ cdef class Vertex(object): new = Vertex() return new - cpdef bint equivalent(self, Vertex other) except -2: + cpdef bint equivalent(self, Vertex other, bint strict=True) except -2: """ Return :data:`True` if two vertices `self` and `other` are semantically equivalent, or :data:`False` if not. You should reimplement this @@ -495,20 +495,30 @@ cdef class Graph(object): else: self.vertices = self.ordered_vertices - cpdef bint isIsomorphic(self, Graph other, dict initialMap=None, bint saveOrder=False) except -2: + cpdef bint isIsomorphic(self, Graph other, dict initialMap=None, bint saveOrder=False, bint strict=True) except -2: """ Returns :data:`True` if two graphs are isomorphic and :data:`False` otherwise. Uses the VF2 algorithm of Vento and Foggia. + + Args: + initialMap (dict, optional): initial atom mapping to use + saveOrder (bool, optional): if ``True``, reset atom order after performing atom isomorphism + strict (bool, optional): if ``False``, perform isomorphism ignoring electrons """ - return vf2.isIsomorphic(self, other, initialMap, saveOrder=saveOrder) + return vf2.isIsomorphic(self, other, initialMap, saveOrder=saveOrder, strict=strict) - cpdef list findIsomorphism(self, Graph other, dict initialMap=None, bint saveOrder=False): + cpdef list findIsomorphism(self, Graph other, dict initialMap=None, bint saveOrder=False, bint strict=True): """ Returns :data:`True` if `other` is subgraph isomorphic and :data:`False` otherwise, and the matching mapping. Uses the VF2 algorithm of Vento and Foggia. + + Args: + initialMap (dict, optional): initial atom mapping to use + saveOrder (bool, optional): if ``True``, reset atom order after performing atom isomorphism + strict (bool, optional): if ``False``, perform isomorphism ignoring electrons """ - return vf2.findIsomorphism(self, other, initialMap, saveOrder=saveOrder) + return vf2.findIsomorphism(self, other, initialMap, saveOrder=saveOrder, strict=strict) cpdef bint isSubgraphIsomorphic(self, Graph other, dict initialMap=None, bint saveOrder=False) except -2: """ diff --git a/rmgpy/molecule/group.pxd b/rmgpy/molecule/group.pxd index bd89000b31..fab5a3ae7b 100644 --- a/rmgpy/molecule/group.pxd +++ b/rmgpy/molecule/group.pxd @@ -63,7 +63,7 @@ cdef class GroupAtom(Vertex): cpdef applyAction(self, list action) - cpdef bint equivalent(self, Vertex other) except -2 + cpdef bint equivalent(self, Vertex other, bint strict=?) except -2 cpdef bint isSpecificCaseOf(self, Vertex other) except -2 @@ -168,9 +168,9 @@ cdef class Group(Graph): cpdef update_charge(self) - cpdef bint isIsomorphic(self, Graph other, dict initialMap=?, bint saveOrder=?) except -2 + cpdef bint isIsomorphic(self, Graph other, dict initialMap=?, bint saveOrder=?, bint strict=?) except -2 - cpdef list findIsomorphism(self, Graph other, dict initialMap=?, bint saveOrder=?) + cpdef list findIsomorphism(self, Graph other, dict initialMap=?, bint saveOrder=?, bint strict=?) cpdef bint isSubgraphIsomorphic(self, Graph other, dict initialMap=?, bint generateInitialMap=?, bint saveOrder=?) except -2 diff --git a/rmgpy/molecule/group.py b/rmgpy/molecule/group.py index 8bc19c00da..d2730920f3 100644 --- a/rmgpy/molecule/group.py +++ b/rmgpy/molecule/group.py @@ -328,7 +328,7 @@ def applyAction(self, action): else: raise ActionError('Unable to update GroupAtom: Invalid action {0}".'.format(action)) - def equivalent(self, other): + def equivalent(self, other, strict=True): """ Returns ``True`` if `other` is equivalent to `self` or ``False`` if not, where `other` can be either an :class:`Atom` or an :class:`GroupAtom` @@ -337,6 +337,8 @@ def equivalent(self, other): """ cython.declare(group=GroupAtom) + if not strict: + raise NotImplementedError('There is currently no implementation of the strict argument for Group objects.') if not isinstance(other, GroupAtom): # Let the equivalent method of other handle it # We expect self to be an Atom object, but can't test for it here @@ -1651,7 +1653,7 @@ def updateFingerprint(self): if len(atom.radicalElectrons) >= 1: self.radicalCount += atom.radicalElectrons[0] - def isIsomorphic(self, other, initialMap=None, saveOrder=False): + def isIsomorphic(self, other, initialMap=None, saveOrder=False, strict=True): """ Returns ``True`` if two graphs are isomorphic and ``False`` otherwise. The `initialMap` attribute can be used to specify a required @@ -1659,6 +1661,8 @@ def isIsomorphic(self, other, initialMap=None, saveOrder=False): while the atoms of `other` are the values). The `other` parameter must be a :class:`Group` object, or a :class:`TypeError` is raised. """ + if not strict: + raise NotImplementedError('There is currently no implementation of the strict argument for Group objects.') # It only makes sense to compare a Group to a Group for full # isomorphism, so raise an exception if this is not what was requested if not isinstance(other, Group): @@ -1666,7 +1670,7 @@ def isIsomorphic(self, other, initialMap=None, saveOrder=False): # Do the isomorphism comparison return Graph.isIsomorphic(self, other, initialMap, saveOrder=saveOrder) - def findIsomorphism(self, other, initialMap=None, saveOrder=False): + def findIsomorphism(self, other, initialMap=None, saveOrder=False, strict=True): """ Returns ``True`` if `other` is isomorphic and ``False`` otherwise, and the matching mapping. The `initialMap` attribute can be @@ -1676,6 +1680,8 @@ def findIsomorphism(self, other, initialMap=None, saveOrder=False): and the atoms of `other` for the values. The `other` parameter must be a :class:`Group` object, or a :class:`TypeError` is raised. """ + if not strict: + raise NotImplementedError('There is currently no implementation of the strict argument for Group objects.') # It only makes sense to compare a Group to a Group for full # isomorphism, so raise an exception if this is not what was requested if not isinstance(other, Group): diff --git a/rmgpy/molecule/molecule.pxd b/rmgpy/molecule/molecule.pxd index 5696a2661e..f76268afeb 100644 --- a/rmgpy/molecule/molecule.pxd +++ b/rmgpy/molecule/molecule.pxd @@ -47,7 +47,7 @@ cdef class Atom(Vertex): cdef public int id cdef public dict props - cpdef bint equivalent(self, Vertex other) except -2 + cpdef bint equivalent(self, Vertex other, bint strict=?) except -2 cpdef bint isSpecificCaseOf(self, Vertex other) except -2 @@ -192,9 +192,9 @@ cdef class Molecule(Graph): cpdef dict get_element_count(self) - cpdef bint isIsomorphic(self, Graph other, dict initialMap=?, bint generateInitialMap=?, bint saveOrder=?) except -2 + cpdef bint isIsomorphic(self, Graph other, dict initialMap=?, bint generateInitialMap=?, bint saveOrder=?, bint strict=?) except -2 - cpdef list findIsomorphism(self, Graph other, dict initialMap=?, bint saveOrder=?) + cpdef list findIsomorphism(self, Graph other, dict initialMap=?, bint saveOrder=?, bint strict=?) cpdef bint isSubgraphIsomorphic(self, Graph other, dict initialMap=?, bint generateInitialMap=?, bint saveOrder=?) except -2 diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index c9301c9b90..f5deb1cefc 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -168,26 +168,31 @@ def symbol(self): return self.element.symbol @property def bonds(self): return self.edges - def equivalent(self, other): + def equivalent(self, other, strict=True): """ Return ``True`` if `other` is indistinguishable from this atom, or ``False`` otherwise. If `other` is an :class:`Atom` object, then all attributes except `label` and 'ID' must match exactly. If `other` is an :class:`GroupAtom` object, then the atom must match any of the - combinations in the atom pattern. + combinations in the atom pattern. If ``strict`` is ``False``, then only + the element is compared and electrons are ignored. """ cython.declare(atom=Atom, ap=gr.GroupAtom) if isinstance(other, Atom): atom = other - return ( - self.element is atom.element and - self.radicalElectrons == atom.radicalElectrons and - self.lonePairs == atom.lonePairs and - self.charge == atom.charge and - self.atomType is atom.atomType - ) + if strict: + return (self.element is atom.element + and self.radicalElectrons == atom.radicalElectrons + and self.lonePairs == atom.lonePairs + and self.charge == atom.charge + and self.atomType is atom.atomType) + else: + return self.element is atom.element elif isinstance(other, gr.GroupAtom): cython.declare(a=AtomType, radical=cython.short, lp=cython.short, charge=cython.short) + if not strict: + raise NotImplementedError('There is currently no implementation of ' + 'the strict argument for Group objects.') ap = other for a in ap.atomType: if self.atomType.equivalent(a): break @@ -1319,7 +1324,7 @@ def get_element_count(self): return element_count - def isIsomorphic(self, other, initialMap=None, generateInitialMap=False, saveOrder=False): + def isIsomorphic(self, other, initialMap=None, generateInitialMap=False, saveOrder=False, strict=True): """ Returns :data:`True` if two graphs are isomorphic and :data:`False` otherwise. The `initialMap` attribute can be used to specify a required @@ -1327,6 +1332,12 @@ def isIsomorphic(self, other, initialMap=None, generateInitialMap=False, saveOrd while the atoms of `other` are the values). The `other` parameter must be a :class:`Molecule` object, or a :class:`TypeError` is raised. Also ensures multiplicities are also equal. + + Args: + initialMap (dict, optional): initial atom mapping to use + generateInitialMap (bool, optional): if ``True``, initialize map by pairing atoms with same labels + saveOrder (bool, optional): if ``True``, reset atom order after performing atom isomorphism + strict (bool, optional): if ``False``, perform isomorphism ignoring electrons """ # It only makes sense to compare a Molecule to a Molecule for full # isomorphism, so raise an exception if this is not what was requested @@ -1355,10 +1366,10 @@ def isIsomorphic(self, other, initialMap=None, generateInitialMap=False, saveOrd return False # Do the full isomorphism comparison - result = Graph.isIsomorphic(self, other, initialMap, saveOrder=saveOrder) + result = Graph.isIsomorphic(self, other, initialMap, saveOrder=saveOrder, strict=strict) return result - def findIsomorphism(self, other, initialMap=None, saveOrder=False): + def findIsomorphism(self, other, initialMap=None, saveOrder=False, strict=True): """ Returns :data:`True` if `other` is isomorphic and :data:`False` otherwise, and the matching mapping. The `initialMap` attribute can be @@ -1367,6 +1378,11 @@ def findIsomorphism(self, other, initialMap=None, saveOrder=False): values). The returned mapping also uses the atoms of `self` for the keys and the atoms of `other` for the values. The `other` parameter must be a :class:`Molecule` object, or a :class:`TypeError` is raised. + + Args: + initialMap (dict, optional): initial atom mapping to use + saveOrder (bool, optional): if ``True``, reset atom order after performing atom isomorphism + strict (bool, optional): if ``False``, perform isomorphism ignoring electrons """ # It only makes sense to compare a Molecule to a Molecule for full # isomorphism, so raise an exception if this is not what was requested @@ -1382,7 +1398,7 @@ def findIsomorphism(self, other, initialMap=None, saveOrder=False): return [] # Do the isomorphism comparison - result = Graph.findIsomorphism(self, other, initialMap, saveOrder=saveOrder) + result = Graph.findIsomorphism(self, other, initialMap, saveOrder=saveOrder, strict=strict) return result def isSubgraphIsomorphic(self, other, initialMap=None, generateInitialMap=False,saveOrder=False): diff --git a/rmgpy/molecule/moleculeTest.py b/rmgpy/molecule/moleculeTest.py index 15ffc6c8de..8d97f0e513 100644 --- a/rmgpy/molecule/moleculeTest.py +++ b/rmgpy/molecule/moleculeTest.py @@ -1130,6 +1130,30 @@ def testSubgraphIsomorphismRings(self): mapping = molecule.findSubgraphIsomorphisms(groupRing) self.assertEqual(len(mapping), 5) + def test_lax_isomorphism(self): + """Test that we can do isomorphism comparison with strict=False""" + mol1 = Molecule().fromAdjacencyList(""" +multiplicity 2 +1 O u0 p2 c0 {3,D} +2 C u1 p0 c0 {3,S} {4,S} {5,S} +3 C u0 p0 c0 {1,D} {2,S} {6,S} +4 H u0 p0 c0 {2,S} +5 H u0 p0 c0 {2,S} +6 H u0 p0 c0 {3,S} + """) + + mol2 = Molecule().fromAdjacencyList(""" +multiplicity 2 +1 O u1 p2 c0 {3,S} +2 C u0 p0 c0 {3,D} {4,S} {5,S} +3 C u0 p0 c0 {1,S} {2,D} {6,S} +4 H u0 p0 c0 {2,S} +5 H u0 p0 c0 {2,S} +6 H u0 p0 c0 {3,S} + """) + + self.assertTrue(mol1.isIsomorphic(mol2, strict=False)) + def testAdjacencyList(self): """ Check the adjacency list read/write functions for a full molecule. diff --git a/rmgpy/molecule/vf2.pxd b/rmgpy/molecule/vf2.pxd index 86a09983f8..409af45dbf 100644 --- a/rmgpy/molecule/vf2.pxd +++ b/rmgpy/molecule/vf2.pxd @@ -36,19 +36,20 @@ cdef class VF2: cdef dict initialMapping cdef bint subgraph cdef bint findAll + cdef bint strict cdef bint isMatch cdef list mappingList - cpdef bint isIsomorphic(self, Graph graph1, Graph graph2, dict initialMapping, bint saveOrder=?) except -2 + cpdef bint isIsomorphic(self, Graph graph1, Graph graph2, dict initialMapping, bint saveOrder=?, bint strict=?) except -2 - cpdef list findIsomorphism(self, Graph graph1, Graph graph2, dict initialMapping, bint saveOrder=?) + cpdef list findIsomorphism(self, Graph graph1, Graph graph2, dict initialMapping, bint saveOrder=?, bint strict=?) cpdef bint isSubgraphIsomorphic(self, Graph graph1, Graph graph2, dict initialMapping, bint saveOrder=?) except -2 cpdef list findSubgraphIsomorphisms(self, Graph graph1, Graph graph2, dict initialMapping, bint saveOrder=?) - cdef isomorphism(self, Graph graph1, Graph graph2, dict initialMapping, bint subgraph, bint findAll, bint saveOrder=?) + cdef isomorphism(self, Graph graph1, Graph graph2, dict initialMapping, bint subgraph, bint findAll, bint saveOrder=?, bint strict=?) cdef bint match(self, int callDepth) except -2 diff --git a/rmgpy/molecule/vf2.pyx b/rmgpy/molecule/vf2.pyx index 41ba85f419..7378865fe8 100644 --- a/rmgpy/molecule/vf2.pyx +++ b/rmgpy/molecule/vf2.pyx @@ -63,22 +63,22 @@ cdef class VF2: self.graph2 = value self.graph2.sortVertices() - cpdef bint isIsomorphic(self, Graph graph1, Graph graph2, dict initialMapping, bint saveOrder=False) except -2: + cpdef bint isIsomorphic(self, Graph graph1, Graph graph2, dict initialMapping, bint saveOrder=False, bint strict=True) except -2: """ Return ``True`` if graph `graph1` is isomorphic to graph `graph2` with the optional initial mapping `initialMapping`, or ``False`` otherwise. """ - self.isomorphism(graph1, graph2, initialMapping, False, False, saveOrder) + self.isomorphism(graph1, graph2, initialMapping, False, False, saveOrder=saveOrder, strict=strict) return self.isMatch - cpdef list findIsomorphism(self, Graph graph1, Graph graph2, dict initialMapping, bint saveOrder=False): + cpdef list findIsomorphism(self, Graph graph1, Graph graph2, dict initialMapping, bint saveOrder=False, bint strict=True): """ Return a list of dicts of all valid isomorphism mappings from graph `graph1` to graph `graph2` with the optional initial mapping `initialMapping`. If no valid isomorphisms are found, an empty list is returned. """ - self.isomorphism(graph1, graph2, initialMapping, False, True, saveOrder) + self.isomorphism(graph1, graph2, initialMapping, False, True, saveOrder=saveOrder, strict=strict) return self.mappingList cpdef bint isSubgraphIsomorphic(self, Graph graph1, Graph graph2, dict initialMapping, bint saveOrder=False) except -2: @@ -100,7 +100,7 @@ cdef class VF2: self.isomorphism(graph1, graph2, initialMapping, True, True, saveOrder) return self.mappingList - cdef isomorphism(self, Graph graph1, Graph graph2, dict initialMapping, bint subgraph, bint findAll, bint saveOrder=False): + cdef isomorphism(self, Graph graph1, Graph graph2, dict initialMapping, bint subgraph, bint findAll, bint saveOrder=False, bint strict=True): """ Evaluate the isomorphism relationship between graphs `graph1` and `graph2` with optional initial mapping `initialMapping`. If `subgraph` @@ -121,6 +121,7 @@ cdef class VF2: self.initialMapping = initialMapping self.subgraph = subgraph self.findAll = findAll + self.strict = strict # Clear previous result self.isMatch = False @@ -285,7 +286,7 @@ cdef class VF2: if self.subgraph: if not vertex1.isSpecificCaseOf(vertex2): return False else: - if not vertex1.equivalent(vertex2): return False + if not vertex1.equivalent(vertex2, strict=self.strict): return False # Semantic check #2: adjacent vertices to vertex1 and vertex2 that are # already mapped should be connected by equivalent edges @@ -295,12 +296,15 @@ cdef class VF2: if vert1 not in vertex1.edges: # The vertices are joined in graph2, but not in graph1 return False - edge1 = vertex1.edges[vert1] - edge2 = vertex2.edges[vert2] - if self.subgraph: - if not edge1.isSpecificCaseOf(edge2): return False - else: - if not edge1.equivalent(edge2): return False + if self.strict: + # Check that the edges are equivalent + # If self.strict=False, we only care that the edge exists + edge1 = vertex1.edges[vert1] + edge2 = vertex2.edges[vert2] + if self.subgraph: + if not edge1.isSpecificCaseOf(edge2): return False + else: + if not edge1.equivalent(edge2): return False # There could still be edges in graph1 that aren't in graph2; this is okay # for subgraph matching, but not for exact matching From da870ca09b74c35eeffe912a83ac80edb79ed36d Mon Sep 17 00:00:00 2001 From: Max Liu Date: Wed, 3 Apr 2019 18:18:10 -0400 Subject: [PATCH 08/20] Add strict argument to Species.isIsomorphic --- rmgpy/species.pxd | 2 +- rmgpy/species.py | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rmgpy/species.pxd b/rmgpy/species.pxd index c24f2abb92..69074e8da9 100644 --- a/rmgpy/species.pxd +++ b/rmgpy/species.pxd @@ -58,7 +58,7 @@ cdef class Species: cpdef generate_resonance_structures(self, bint keep_isomorphic=?, bint filter_structures=?) - cpdef bint isIsomorphic(self, other, bint generate_res=?, bint generateInitialMap=?) except -2 + cpdef bint isIsomorphic(self, other, bint generate_res=?, bint generateInitialMap=?, bint strict=?) except -2 cpdef bint isIdentical(self, other) except -2 diff --git a/rmgpy/species.py b/rmgpy/species.py index 0dcf1c528b..a9c5398404 100644 --- a/rmgpy/species.py +++ b/rmgpy/species.py @@ -204,30 +204,30 @@ def generate_resonance_structures(self, keep_isomorphic=True, filter_structures= self.molecule = self.molecule[0].generate_resonance_structures(keep_isomorphic=keep_isomorphic, filter_structures=filter_structures) - def isIsomorphic(self, other, generate_res=False, generateInitialMap=False): + def isIsomorphic(self, other, generate_res=False, generateInitialMap=False, strict=True): """ Return ``True`` if the species is isomorphic to `other`, which can be either a :class:`Molecule` object or a :class:`Species` object. - If generate_res is ``True`` and other is a :class:`Species` object, the resonance structures of other will - be generated and isomorphically compared against self. This is useful for situations where a - "non-representative" resonance structure of self is generated, and it should be identified as the same Species, - and be assigned a reactive=False flag. + + Args: + generateInitialMap (bool, optional): If ``True``, make initial map by matching labeled atoms + strict (bool, optional): If ``False``, perform isomorphism ignoring electrons. """ if isinstance(other, Molecule): for molecule in self.molecule: - if molecule.isIsomorphic(other,generateInitialMap=generateInitialMap): + if molecule.isIsomorphic(other, generateInitialMap=generateInitialMap, strict=strict): return True elif isinstance(other, Species): for molecule1 in self.molecule: for molecule2 in other.molecule: - if molecule1.isIsomorphic(molecule2,generateInitialMap=generateInitialMap): + if molecule1.isIsomorphic(molecule2, generateInitialMap=generateInitialMap, strict=strict): return True if generate_res: other_copy = other.copy(deep=True) other_copy.generate_resonance_structures(keep_isomorphic=False) for molecule1 in self.molecule: for molecule2 in other_copy.molecule: - if molecule1.isIsomorphic(molecule2,generateInitialMap=generateInitialMap): + if molecule1.isIsomorphic(molecule2, generateInitialMap=generateInitialMap): # If they are isomorphic and this was found only by generating resonance structures, append # the structure in other to self.molecule as unreactive, since it is a non-representative # resonance structure of it, and return `True`. From e36c78e7927305b8e893628cf66d7391e58d4206 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 4 Apr 2019 14:35:06 -0400 Subject: [PATCH 09/20] Remove generate_res argument from Species.isIsomorphic Because its functionality is replaced by the strict argument --- rmgpy/species.pxd | 2 +- rmgpy/species.py | 14 +------------- rmgpy/speciesTest.py | 14 +++++++------- 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/rmgpy/species.pxd b/rmgpy/species.pxd index 69074e8da9..0e126d1281 100644 --- a/rmgpy/species.pxd +++ b/rmgpy/species.pxd @@ -58,7 +58,7 @@ cdef class Species: cpdef generate_resonance_structures(self, bint keep_isomorphic=?, bint filter_structures=?) - cpdef bint isIsomorphic(self, other, bint generate_res=?, bint generateInitialMap=?, bint strict=?) except -2 + cpdef bint isIsomorphic(self, other, bint generateInitialMap=?, bint strict=?) except -2 cpdef bint isIdentical(self, other) except -2 diff --git a/rmgpy/species.py b/rmgpy/species.py index a9c5398404..9911f6afdd 100644 --- a/rmgpy/species.py +++ b/rmgpy/species.py @@ -204,7 +204,7 @@ def generate_resonance_structures(self, keep_isomorphic=True, filter_structures= self.molecule = self.molecule[0].generate_resonance_structures(keep_isomorphic=keep_isomorphic, filter_structures=filter_structures) - def isIsomorphic(self, other, generate_res=False, generateInitialMap=False, strict=True): + def isIsomorphic(self, other, generateInitialMap=False, strict=True): """ Return ``True`` if the species is isomorphic to `other`, which can be either a :class:`Molecule` object or a :class:`Species` object. @@ -222,18 +222,6 @@ def isIsomorphic(self, other, generate_res=False, generateInitialMap=False, stri for molecule2 in other.molecule: if molecule1.isIsomorphic(molecule2, generateInitialMap=generateInitialMap, strict=strict): return True - if generate_res: - other_copy = other.copy(deep=True) - other_copy.generate_resonance_structures(keep_isomorphic=False) - for molecule1 in self.molecule: - for molecule2 in other_copy.molecule: - if molecule1.isIsomorphic(molecule2, generateInitialMap=generateInitialMap): - # If they are isomorphic and this was found only by generating resonance structures, append - # the structure in other to self.molecule as unreactive, since it is a non-representative - # resonance structure of it, and return `True`. - other_copy.molecule[0].reactive = False - self.molecule.append(other_copy.molecule[0]) - return True else: raise ValueError('Unexpected value "{0!r}" for other parameter; should be a Molecule or Species object.'.format(other)) return False diff --git a/rmgpy/speciesTest.py b/rmgpy/speciesTest.py index e55f64e45f..ec8d507e7d 100644 --- a/rmgpy/speciesTest.py +++ b/rmgpy/speciesTest.py @@ -226,15 +226,15 @@ def test_is_isomorphic_to_filtered_resonance_structure(self): 5 O u0 p3 c-1 {3,S}""") # check that the structures are not isomorphic if resonance structures are not generated: - self.assertFalse(spc1_correct.isIsomorphic(spc1_nonrepresentative, generate_res=False)) + self.assertFalse(spc1_correct.isIsomorphic(spc1_nonrepresentative, strict=True)) # check that the nonrepresentative structure is isomorphic by generating resonance structures: - self.assertTrue(spc1_correct.isIsomorphic(spc1_nonrepresentative, generate_res=True)) - self.assertTrue(spc2_correct.isIsomorphic(spc2_nonrepresentative, generate_res=True)) - self.assertTrue(spc3_correct.isIsomorphic(spc3_nonrepresentative, generate_res=True)) - self.assertTrue(spc4_correct.isIsomorphic(spc4_nonrepresentative, generate_res=True)) - self.assertTrue(spc5_correct.isIsomorphic(spc5_nonrepresentative, generate_res=True)) - self.assertTrue(spc6_correct.isIsomorphic(spc6_nonrepresentative, generate_res=True)) + self.assertTrue(spc1_correct.isIsomorphic(spc1_nonrepresentative, strict=False)) + self.assertTrue(spc2_correct.isIsomorphic(spc2_nonrepresentative, strict=False)) + self.assertTrue(spc3_correct.isIsomorphic(spc3_nonrepresentative, strict=False)) + self.assertTrue(spc4_correct.isIsomorphic(spc4_nonrepresentative, strict=False)) + self.assertTrue(spc5_correct.isIsomorphic(spc5_nonrepresentative, strict=False)) + self.assertTrue(spc6_correct.isIsomorphic(spc6_nonrepresentative, strict=False)) def testGetResonanceHybrid(self): """ From 4422e9cd4d8122efa6b59dcd8a8b923cbd638d99 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Wed, 3 Apr 2019 18:24:45 -0400 Subject: [PATCH 10/20] Rename isomorphic_species_lists to same_species_lists Also add strict argument which is passed to isIsomorphic --- rmgpy/data/kinetics/database.py | 10 ++--- rmgpy/data/kinetics/family.py | 8 ++-- rmgpy/reaction.pxd | 2 +- rmgpy/reaction.py | 78 +++++++++++++++++++-------------- rmgpy/reactionTest.py | 2 +- rmgpy/tools/isotopes.py | 11 +++-- 6 files changed, 60 insertions(+), 51 deletions(-) diff --git a/rmgpy/data/kinetics/database.py b/rmgpy/data/kinetics/database.py index 1bdd66fd0d..1eeedbdbb9 100644 --- a/rmgpy/data/kinetics/database.py +++ b/rmgpy/data/kinetics/database.py @@ -41,7 +41,7 @@ StickingCoefficientBEP, SurfaceArrhenius, SurfaceArrheniusBEP from rmgpy.molecule import Molecule, Group from rmgpy.species import Species -from rmgpy.reaction import Reaction, isomorphic_species_lists +from rmgpy.reaction import Reaction, same_species_lists from rmgpy.data.base import LogicNode from .family import KineticsFamily @@ -620,11 +620,11 @@ def getForwardReactionForFamilyEntry(self, entry, family, thermoDatabase): # Remove from that set any reactions that don't produce the desired reactants and products forward = []; reverse = [] for rxn in generatedReactions: - if (isomorphic_species_lists(reaction.reactants, rxn.reactants) - and isomorphic_species_lists(reaction.products, rxn.products)): + if (same_species_lists(reaction.reactants, rxn.reactants) + and same_species_lists(reaction.products, rxn.products)): forward.append(rxn) - if (isomorphic_species_lists(reaction.reactants, rxn.products) - and isomorphic_species_lists(reaction.products, rxn.reactants)): + if (same_species_lists(reaction.reactants, rxn.products) + and same_species_lists(reaction.products, rxn.reactants)): reverse.append(rxn) # We should now know whether the reaction is given in the forward or diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index 46fad554d3..319d94cf6a 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -44,7 +44,7 @@ from rmgpy.constraints import failsSpeciesConstraints from rmgpy.data.base import Database, Entry, LogicNode, LogicOr, ForbiddenStructures,\ getAllCombinations -from rmgpy.reaction import Reaction, isomorphic_species_lists +from rmgpy.reaction import Reaction, same_species_lists from rmgpy import settings from rmgpy.reaction import Reaction from rmgpy.kinetics import Arrhenius, SurfaceArrhenius,\ @@ -1570,7 +1570,7 @@ def __createReaction(self, reactants, products, is_forward): """ # Make sure the products are in fact different than the reactants - if isomorphic_species_lists(reactants, products): + if same_species_lists(reactants, products): return None # Create and return template reaction object @@ -2146,7 +2146,7 @@ def generate_products_and_reactions(order): products0[i] = aromaticStructs[0] # Skip reactions that don't match the given products - if isomorphic_species_lists(products, products0): + if same_species_lists(products, products0): rxnList.append(reaction) # Determine the reactant-product pairs to use for flux analysis @@ -2590,7 +2590,7 @@ def getLabeledReactantsAndProducts(self, reactants, products): pass else: if product_structures is not None: - if isomorphic_species_lists(list(products), list(product_structures)): + if same_species_lists(list(products), list(product_structures)): return reactant_structures, product_structures else: continue diff --git a/rmgpy/reaction.pxd b/rmgpy/reaction.pxd index 8ff94c31cc..d71402aa9c 100644 --- a/rmgpy/reaction.pxd +++ b/rmgpy/reaction.pxd @@ -126,4 +126,4 @@ cdef class Reaction: cpdef get_mean_sigma_and_epsilon(self, bint reverse=?) -cpdef bint isomorphic_species_lists(list list1, list list2, bint check_identical=?, bint only_check_label=?, bint generateInitialMap=?) +cpdef bint same_species_lists(list list1, list list2, bint check_identical=?, bint only_check_label=?, bint generate_initial_map=?, bint strict=?) except -2 diff --git a/rmgpy/reaction.py b/rmgpy/reaction.py index 0f4e0ab7c9..471c3b746f 100644 --- a/rmgpy/reaction.py +++ b/rmgpy/reaction.py @@ -420,13 +420,13 @@ def matchesSpecies(self, reactants, products=None): products (list, optional): Species required on the other side """ # Check forward direction - if isomorphic_species_lists(self.reactants, reactants): - if products is None or isomorphic_species_lists(self.products, products): + if same_species_lists(self.reactants, reactants): + if products is None or same_species_lists(self.products, products): return True else: return False - elif isomorphic_species_lists(self.products, reactants): - if products is None or isomorphic_species_lists(self.reactants, products): + elif same_species_lists(self.products, reactants): + if products is None or same_species_lists(self.reactants, products): return True else: return False @@ -459,19 +459,22 @@ def isIsomorphic(self, other, eitherDirection=True, checkIdentical = False, except AttributeError: raise TypeError('Only use checkTemplateRxnProducts flag for TemplateReactions.') - return isomorphic_species_lists(species1, species2, - check_identical=checkIdentical, - only_check_label=checkOnlyLabel,generateInitialMap=generateInitialMap) + return same_species_lists(species1, species2, + check_identical=checkIdentical, + only_check_label=checkOnlyLabel, + generate_initial_map=generateInitialMap) # Compare reactants to reactants - forwardReactantsMatch = isomorphic_species_lists(self.reactants, other.reactants, - check_identical=checkIdentical, - only_check_label=checkOnlyLabel,generateInitialMap=generateInitialMap) + forwardReactantsMatch = same_species_lists(self.reactants, other.reactants, + check_identical=checkIdentical, + only_check_label=checkOnlyLabel, + generate_initial_map=generateInitialMap) # Compare products to products - forwardProductsMatch = isomorphic_species_lists(self.products, other.products, - check_identical=checkIdentical, - only_check_label=checkOnlyLabel,generateInitialMap=generateInitialMap) + forwardProductsMatch = same_species_lists(self.products, other.products, + check_identical=checkIdentical, + only_check_label=checkOnlyLabel, + generate_initial_map=generateInitialMap) # Compare specificCollider to specificCollider ColliderMatch = (self.specificCollider == other.specificCollider) @@ -483,17 +486,19 @@ def isIsomorphic(self, other, eitherDirection=True, checkIdentical = False, return False # Compare reactants to products - reverseReactantsMatch = isomorphic_species_lists(self.reactants, other.products, - check_identical=checkIdentical, - only_check_label=checkOnlyLabel,generateInitialMap=generateInitialMap) + reverseReactantsMatch = same_species_lists(self.reactants, other.products, + check_identical=checkIdentical, + only_check_label=checkOnlyLabel, + generate_initial_map=generateInitialMap) # Compare products to reactants - reverseProductsMatch = isomorphic_species_lists(self.products, other.reactants, - check_identical=checkIdentical, - only_check_label=checkOnlyLabel,generateInitialMap=generateInitialMap) + reverseProductsMatch = same_species_lists(self.products, other.reactants, + check_identical=checkIdentical, + only_check_label=checkOnlyLabel, + generate_initial_map=generateInitialMap) # should have already returned if it matches forwards, or we're not allowed to match backwards - return (reverseReactantsMatch and reverseProductsMatch and ColliderMatch) + return reverseReactantsMatch and reverseProductsMatch and ColliderMatch def getEnthalpyOfReaction(self, T): """ @@ -1321,28 +1326,33 @@ def get_mean_sigma_and_epsilon(self, reverse=False): mean_epsilons = reduce((lambda x, y: x * y), epsilons) ** (1 / len(epsilons)) return mean_sigmas, mean_epsilons -def isomorphic_species_lists(list1, list2, check_identical=False, only_check_label=False, generateInitialMap=False): + +def same_species_lists(list1, list2, check_identical=False, only_check_label=False, generate_initial_map=False, strict=True): """ - This method compares whether lists of species or molecules are isomorphic - or identical. It is used for the 'isIsomorphic' method of Reaction class. - It likely can be useful elswehere as well: - - list1 - list of species/molecule objects of reaction1 - list2 - list of species/molecule objects of reaction2 - check_identical - if true, uses the 'isIdentical' comparison - if false, uses the 'isIsomorphic' comparison - only_check_label - only look at species' labels, no isomorphism checks - - Returns True if the lists are isomorphic/identical & false otherwise + This method compares whether two lists of species or molecules are the same, + given the comparison options provided. It is used for the `is_same` method + of :class:`Reaction`, but may also be useful in other situations. + + Args: + list1 (list): list of :class:`Species` or :class:`Molecule` objects + list2 (list): list of :class:`Species` or :class:`Molecule` objects + check_identical (bool, optional): if ``True``, use isIdentical comparison and compare atom IDs + only_check_label (bool, optional): if ``True``, only compare the label attribute of each species + generate_initial_map (bool, optional): if ``True``, initialize map by pairing atoms with same labels + strict (bool, optional): if ``False``, perform isomorphism ignoring electrons + + Returns: + ``True`` if the lists are the same and ``False`` otherwise """ - def same(object1, object2, _check_identical=check_identical, _only_check_label=only_check_label, _generate_initial_map=generateInitialMap): + def same(object1, object2, _check_identical=check_identical, _only_check_label=only_check_label, + _generate_initial_map=generate_initial_map, _strict=strict): if _only_check_label: return str(object1) == str(object2) elif _check_identical: return object1.isIdentical(object2) else: - return object1.isIsomorphic(object2,generateInitialMap=_generate_initial_map) + return object1.isIsomorphic(object2, generateInitialMap=_generate_initial_map, strict=_strict) if len(list1) == len(list2) == 1: if same(list1[0], list2[0]): diff --git a/rmgpy/reactionTest.py b/rmgpy/reactionTest.py index 73ae3e71bd..9f450627d2 100644 --- a/rmgpy/reactionTest.py +++ b/rmgpy/reactionTest.py @@ -65,7 +65,7 @@ def __repr__(self): return "PseudoSpecies('{0}')".format(self.label) def __str__(self): return self.label - def isIsomorphic(self, other, generateInitialMap=False): + def isIsomorphic(self, other, generateInitialMap=False, strict=True): return self.label.lower() == other.label.lower() class TestReactionIsomorphism(unittest.TestCase): diff --git a/rmgpy/tools/isotopes.py b/rmgpy/tools/isotopes.py index 329c7df4fc..3dfb1427f0 100644 --- a/rmgpy/tools/isotopes.py +++ b/rmgpy/tools/isotopes.py @@ -55,7 +55,7 @@ from rmgpy.data.rmg import getDB import rmgpy.molecule.element from rmgpy.kinetics.arrhenius import MultiArrhenius -from rmgpy.reaction import isomorphic_species_lists +from rmgpy.reaction import same_species_lists def initialize_isotope_model(rmg, isotopes): """ @@ -179,9 +179,8 @@ def generate_isotope_reactions(isotopeless_reactions, isotopes): rxns_w_same_reactants = [rxn] rxn_index2 = rxn_index + 1 while rxn_index2 < len(isotopeless_reactions): - if isomorphic_species_lists(isotopeless_reactions[rxn_index].reactants, - isotopeless_reactions[rxn_index2].reactants, - ): + if same_species_lists(isotopeless_reactions[rxn_index].reactants, + isotopeless_reactions[rxn_index2].reactants,): rxns_w_same_reactants.append(isotopeless_reactions[rxn_index2]) del isotopeless_reactions[rxn_index2] else: @@ -210,8 +209,8 @@ def generate_isotope_reactions(isotopeless_reactions, isotopes): while rxn_index3 < len(reactant_pairs): rxn_index4 = rxn_index3 + 1 while rxn_index4 < len(reactant_pairs): - if isomorphic_species_lists(reactant_pairs[rxn_index3], - reactant_pairs[rxn_index4]): + if same_species_lists(reactant_pairs[rxn_index3], + reactant_pairs[rxn_index4]): del reactant_pairs[rxn_index4] else: rxn_index4 += 1 From f2a229696aae029bdbc51509e8ff53862daed823 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Wed, 3 Apr 2019 18:50:07 -0400 Subject: [PATCH 11/20] Add strict argument to Reaction.isIsomorphic --- rmgpy/reaction.pxd | 3 ++- rmgpy/reaction.py | 40 ++++++++++++++++++++++------------------ 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/rmgpy/reaction.pxd b/rmgpy/reaction.pxd index d71402aa9c..c4aeac7291 100644 --- a/rmgpy/reaction.pxd +++ b/rmgpy/reaction.pxd @@ -72,7 +72,8 @@ cdef class Reaction: cpdef bint matchesSpecies(self, list reactants, list products=?) - cpdef bint isIsomorphic(self, Reaction other, bint eitherDirection=?, bint checkIdentical=?, bint checkOnlyLabel=?, bint checkTemplateRxnProducts=?, bint generateInitialMap=?) + cpdef bint isIsomorphic(self, Reaction other, bint eitherDirection=?, bint checkIdentical=?, bint checkOnlyLabel=?, + bint checkTemplateRxnProducts=?, bint generateInitialMap=?, bint strict=?) except -2 cpdef double getEnthalpyOfReaction(self, double T) diff --git a/rmgpy/reaction.py b/rmgpy/reaction.py index 471c3b746f..1b619ec086 100644 --- a/rmgpy/reaction.py +++ b/rmgpy/reaction.py @@ -433,24 +433,23 @@ def matchesSpecies(self, reactants, products=None): else: return False - def isIsomorphic(self, other, eitherDirection=True, checkIdentical = False, - checkOnlyLabel = False, checkTemplateRxnProducts=False, generateInitialMap=False): + def isIsomorphic(self, other, eitherDirection=True, checkIdentical = False, checkOnlyLabel = False, + checkTemplateRxnProducts=False, generateInitialMap=False, strict=True): """ Return ``True`` if this reaction is the same as the `other` reaction, or ``False`` if they are different. The comparison involves comparing isomorphism of reactants and products, and doesn't use any kinetic information. - If `eitherDirection=False` then the directions must match. - - `checkIdentical` indicates that atom ID's must match and is used in - checking degeneracy - `checkOnlyLabel` indicates that the string representation will be - checked, ignoring the molecular structure comparisons - `checkTemplateRxnProducts` indicates that only the products of the - reaction are checked for isomorphism. This is used when - we know the reactants are identical, i.e. in generating - reactions. + Args: + eitherDirection (bool, optional): if ``False``,then the reaction direction must match. + checkIdentical (bool, optional): if ``True``, check that atom ID's match (used for checking degeneracy) + checkOnlyLabel (bool, optional): if ``True``, only check the string representation, + ignoring molecular structure comparisons + checkTemplateRxnProducts (bool, optional): if ``True``, only check isomorphism of reaction products + (used when we know the reactants are identical, i.e. in generating reactions) + generateInitialMap (bool, optional): if ``True``, initialize map by pairing atoms with same labels + strict (bool, optional): if ``False``, perform isomorphism ignoring electrons """ if checkTemplateRxnProducts: try: @@ -462,19 +461,22 @@ def isIsomorphic(self, other, eitherDirection=True, checkIdentical = False, return same_species_lists(species1, species2, check_identical=checkIdentical, only_check_label=checkOnlyLabel, - generate_initial_map=generateInitialMap) + generate_initial_map=generateInitialMap, + strict=strict) # Compare reactants to reactants forwardReactantsMatch = same_species_lists(self.reactants, other.reactants, check_identical=checkIdentical, only_check_label=checkOnlyLabel, - generate_initial_map=generateInitialMap) - + generate_initial_map=generateInitialMap, + strict=strict) + # Compare products to products forwardProductsMatch = same_species_lists(self.products, other.products, check_identical=checkIdentical, only_check_label=checkOnlyLabel, - generate_initial_map=generateInitialMap) + generate_initial_map=generateInitialMap, + strict=strict) # Compare specificCollider to specificCollider ColliderMatch = (self.specificCollider == other.specificCollider) @@ -489,13 +491,15 @@ def isIsomorphic(self, other, eitherDirection=True, checkIdentical = False, reverseReactantsMatch = same_species_lists(self.reactants, other.products, check_identical=checkIdentical, only_check_label=checkOnlyLabel, - generate_initial_map=generateInitialMap) + generate_initial_map=generateInitialMap, + strict=strict) # Compare products to reactants reverseProductsMatch = same_species_lists(self.products, other.reactants, check_identical=checkIdentical, only_check_label=checkOnlyLabel, - generate_initial_map=generateInitialMap) + generate_initial_map=generateInitialMap, + strict=strict) # should have already returned if it matches forwards, or we're not allowed to match backwards return reverseReactantsMatch and reverseProductsMatch and ColliderMatch From 75da1f39f50e67bce466c939baf1776684f9d34f Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 4 Apr 2019 14:44:44 -0400 Subject: [PATCH 12/20] Refactor CERM.checkForExistingSpecies using strict=False isomorphism Since this option allows us to compare resonance structures directly, we don't need to worry about nonreactive resonance structures or aromatic resonance structures. Also, the return value of checkForExistingSpecies is reduced to a single value, since returning whether or not the species was found is redundant with returning the species itself. --- rmgpy/rmg/model.py | 111 ++++++++++------------------------------- rmgpy/rmg/modelTest.py | 9 ++-- rmgpy/rmg/rmgTest.py | 4 +- 3 files changed, 31 insertions(+), 93 deletions(-) diff --git a/rmgpy/rmg/model.py b/rmgpy/rmg/model.py index 71ce29b35b..3af4085573 100644 --- a/rmgpy/rmg/model.py +++ b/rmgpy/rmg/model.py @@ -238,71 +238,35 @@ def __init__(self, core=None, edge=None, surface=None): def checkForExistingSpecies(self, molecule): """ Check to see if an existing species contains the same - :class:`molecule.Molecule` as `molecule`. - Returns ``True``, `reactive`, and the matched species (if found) or - ``False``, ``False``, and ``None`` (if not found). - `reactive` is a boolean argument which is ``False`` if this molecule is an unrepresentative resonance structure - of an existing species (i.e., was found to be isomorphic only by generating its unfiltered resonance structures) - and True otherwise. It is emphasized that `reactive` relates to the :Class:`Molecule` attribute. - """ - # Create obj to check against existing species - # obj can be `Molecule` object or `Species` object - - # For non-cyclic molecules, obj is `Molecule` object - # We expect it to be part of the list of isomers in a species - # object if it has a match - obj = molecule - - # For cyclic molecules, obj is `Species` object and aromatic resonance - # isomers are generated. This is due to the hysteresis of isomer generation - # for aromatic/polyaromatic compounds: not all kekulized forms can be found - # within the list of isomers for a species object describing a unique aromatic compound - if molecule.isCyclic(): - obj = Species(molecule=[molecule]) - from rmgpy.molecule.resonance import generate_optimal_aromatic_resonance_structures - aromaticIsomers = generate_optimal_aromatic_resonance_structures(molecule) - obj.molecule.extend(aromaticIsomers) + :class:`molecule.Molecule` as `molecule`. Comparison is done using + isomorphism without consideration of electrons. Therefore, resonance + structures of a species will all match each other. + + Returns the matched species if found and `None` otherwise. + """ # First check cache and return if species is found for i, spec in enumerate(self.speciesCache): - if spec is not None: - for mol in spec.molecule: - if obj.isIsomorphic(mol): - self.speciesCache.pop(i) - self.speciesCache.insert(0, spec) - return True, True, spec + if spec is not None and spec.isIsomorphic(molecule, strict=False): + self.speciesCache.pop(i) + self.speciesCache.insert(0, spec) + return spec - # Return an existing species if a match is found + # If not found in cache, check all species with matching formula formula = molecule.getFormula() try: - speciesList = self.speciesDict[formula] + species_list = self.speciesDict[formula] except KeyError: - return False, False, None - for spec in speciesList: - if spec.isIsomorphic(obj): - self.speciesCache.pop() - self.speciesCache.insert(0, spec) - return True, True, spec - - # As a last resort, check using molecule.fingerprint if the object matches any existing species, - # and if it does, generate resonance structures w/o filtration and check for isomorphism - candidates = [] - for spec in speciesList: - if spec.molecule[0].fingerprint == molecule.fingerprint: - candidates.append(spec) - if len(candidates) > 0: - mol_copy = molecule.copy(deep=True) - if not mol_copy.reactive: - mol_copy.reactive = True - structures = mol_copy.generate_resonance_structures(keep_isomorphic=False, filter_structures=False) - for spec in candidates: - for mol in spec.molecule: - for structure in structures: - if mol.isIsomorphic(structure): - return True, False, spec - - # At this point we can conclude that the structure does not exist - return False, False, None + pass + else: + for spec in species_list: + if spec.isIsomorphic(molecule, strict=False): + self.speciesCache.pop() + self.speciesCache.insert(0, spec) + return spec + + # At this point we can conclude that the species is new + return None def makeNewSpecies(self, object, label='', reactive=True, checkForExisting=True): """ @@ -324,29 +288,10 @@ def makeNewSpecies(self, object, label='', reactive=True, checkForExisting=True) # If desired, check to ensure that the species is new; return the # existing species if not new if checkForExisting: - if isinstance(object, rmgpy.species.Species) and len(object.molecule) > 1: - # If resonance structures were already generated (e.g., if object came from a reaction library), object - # may contain unreactive resonance structures. Make sure a reactive structure is sent to - # checkForExistingSpecies() - for mol in object.molecule: - if mol.reactive: - found, reactive_structure, spec = self.checkForExistingSpecies(mol) - break - else: - for mol in object.molecule: - logging.info(mol.toAdjacencyList()) - raise AssertionError, "No reactive structures found in species {0}".format(object.molecule[0].toSMILES()) - else: - found, reactive_structure, spec = self.checkForExistingSpecies(molecule) - if found and reactive_structure: - return spec, False - if found and not reactive_structure: - molecule.reactive=False - spec.molecule.append(molecule) + spec = self.checkForExistingSpecies(molecule) + if spec is not None: return spec, False - # Check that the structure is not forbidden - # If we're here then we're ready to make the new species if reactive: self.speciesCounter += 1 # count only reactive species @@ -360,13 +305,7 @@ def makeNewSpecies(self, object, label='', reactive=True, checkForExisting=True) spec = Species(index=speciesIndex, label=label, molecule=[molecule], reactive=reactive) spec.creationIteration = self.iterationNum - if isinstance(object, rmgpy.species.Species) and len(object.molecule) > 1: - # If resonance structures were already generated (e.g., if object came from a reaction library), object may - # contain unreactive resonance structures that we'd like to keep. In this case, don't re-generate the - # resonance structures, just keep the original ones. - spec.molecule = object.molecule - else: - spec.generate_resonance_structures() + spec.generate_resonance_structures() spec.molecularWeight = Quantity(spec.molecule[0].getMolecularWeight()*1000.,"amu") if not spec.thermo: diff --git a/rmgpy/rmg/modelTest.py b/rmgpy/rmg/modelTest.py index 4ce9bbeaef..0a81aa0f68 100644 --- a/rmgpy/rmg/modelTest.py +++ b/rmgpy/rmg/modelTest.py @@ -215,9 +215,7 @@ def testMakeNewSpecies(self): def test_append_unreactive_structure(self): """ - Test that the CoreEdgeReactionModel.makeNewSpecies method correctly appends a non-representative resonance - structure to the correct Species containing the representative resonance structures. - The non-representative structure should be marked as `.reactive=False`. + Test that CERM.makeNewSpecies correctly recognizes a non-representative resonance structure """ cerm = CoreEdgeReactionModel() @@ -233,9 +231,10 @@ def test_append_unreactive_structure(self): self.assertEquals(len(cerm.speciesDict), 2) self.assertEquals(len(cerm.indexSpeciesDict), 2) + self.assertEquals(len(cerm.indexSpeciesDict[1].molecule), 1) self.assertTrue(cerm.indexSpeciesDict[1].molecule[0].reactive) - self.assertNotEquals(cerm.indexSpeciesDict[2].molecule[0].reactive, - cerm.indexSpeciesDict[2].molecule[1].reactive) # only one should be reactive + self.assertEquals(len(cerm.indexSpeciesDict[2].molecule), 1) + self.assertTrue(cerm.indexSpeciesDict[2].molecule[0].reactive) def testMakeNewReaction(self): """ diff --git a/rmgpy/rmg/rmgTest.py b/rmgpy/rmg/rmgTest.py index 1183b2baf8..2ab11dc804 100644 --- a/rmgpy/rmg/rmgTest.py +++ b/rmgpy/rmg/rmgTest.py @@ -172,8 +172,8 @@ def testCheckForExistingSpeciesForBiAromatics(self): 30 H u0 p0 c0 {14,S} 31 H u0 p0 c0 {15,S} """) - found, reactive, spec = rmg_test.reactionModel.checkForExistingSpecies(mol_test) - assert found == True + spec = rmg_test.reactionModel.checkForExistingSpecies(mol_test) + self.assertIsNotNone(spec) def testRestartFileGenerationAndParsing(self): From de012ad906e1a2742ccd1d3be49cc78f7fa96c44 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 13 Sep 2018 11:47:52 -0400 Subject: [PATCH 13/20] Refactor product checking in __generateReactions No need to ensure species - the reactions will always contain Molecules at this point, so either Molecule or Species objects are acceptable for the product list to compare with Aromatic resonance structure generation can be omitted by using the strict=False option, which neglects electrons during the isomorphism comparison --- rmgpy/data/kinetics/family.py | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index 319d94cf6a..e08c386043 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -2129,24 +2129,13 @@ def generate_products_and_reactions(order): # If products is given, remove reactions from the reaction list that # don't generate the given products if products is not None: - ensure_species(products, resonance=prod_resonance) - rxnList0 = rxnList[:] rxnList = [] for reaction in rxnList0: - - products0 = reaction.products[:] if forward else reaction.reactants[:] - - # For aromatics, generate aromatic resonance structures to accurately identify isomorphic species - if prod_resonance: - for i, product in enumerate(products0): - if product.isCyclic: - aromaticStructs = generate_optimal_aromatic_resonance_structures(product) - if aromaticStructs: - products0[i] = aromaticStructs[0] - - # Skip reactions that don't match the given products - if same_species_lists(products, products0): + products0 = reaction.products if forward else reaction.reactants + # Only keep reactions which give the requested products + # If prod_resonance=True, then use strict=False to consider all resonance structures + if same_species_lists(products, products0, strict=not prod_resonance): rxnList.append(reaction) # Determine the reactant-product pairs to use for flux analysis From 9e8a4910dd45fc7eada617f1481e415d8f5e9897 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 13 Sep 2018 13:05:14 -0400 Subject: [PATCH 14/20] Revise test for prod_resonance option for generating reactions Remove checks that the product list is converted in place to Species objects, since that step was removed --- rmgpy/data/kinetics/kineticsTest.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/rmgpy/data/kinetics/kineticsTest.py b/rmgpy/data/kinetics/kineticsTest.py index 0158b4d3b1..f6b2df1ab5 100644 --- a/rmgpy/data/kinetics/kineticsTest.py +++ b/rmgpy/data/kinetics/kineticsTest.py @@ -915,8 +915,6 @@ def test_generate_reactions_from_families_product_resonance(self): self.assertEqual(len(reaction_list), 1) self.assertEqual(reaction_list[0].degeneracy, 2) - - def test_generate_reactions_from_families_product_resonance2(self): """Test that we can specify the no product resonance structure when generating reactions""" reactants = [ @@ -931,9 +929,6 @@ def test_generate_reactions_from_families_product_resonance2(self): reaction_list = self.database.kinetics.generate_reactions_from_families(reactants, products, only_families=['H_Abstraction'], resonance=False) self.assertEqual(len(reaction_list), 0) - self.assertTrue(isinstance(products[0],Species)) - self.assertEqual(len(products[0].molecule),1) - def test_generate_reactions_from_libraries(self): """Test that we can generate reactions from libraries""" reactants = [ From 7b4604eb155c67c2d96bb7d844c66c4eddc50bb4 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 4 Apr 2019 17:48:39 -0400 Subject: [PATCH 15/20] Add strict option to Graph.isMappingValid Similar to strict option for isomorphism, setting strict=False ignores electrons and bond orders for the comparison, which is useful for comparing different resonance structures --- rmgpy/molecule/graph.pxd | 2 +- rmgpy/molecule/graph.pyx | 34 +++++++++++++++++++++------------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/rmgpy/molecule/graph.pxd b/rmgpy/molecule/graph.pxd index 26cd2af6f6..cfa9ea1d52 100644 --- a/rmgpy/molecule/graph.pxd +++ b/rmgpy/molecule/graph.pxd @@ -156,6 +156,6 @@ cdef class Graph(object): cpdef list getLargestRing(self, Vertex vertex) - cpdef bint isMappingValid(self, Graph other, dict mapping, bint equivalent=?) except -2 + cpdef bint isMappingValid(self, Graph other, dict mapping, bint equivalent=?, bint strict=?) except -2 cpdef list get_edges_in_cycle(self, list vertices, bint sort=?) diff --git a/rmgpy/molecule/graph.pyx b/rmgpy/molecule/graph.pyx index 94c554b5aa..4a0c3c3ae1 100644 --- a/rmgpy/molecule/graph.pyx +++ b/rmgpy/molecule/graph.pyx @@ -1075,25 +1075,28 @@ cdef class Graph(object): return longest_cycle - cpdef bint isMappingValid(self, Graph other, dict mapping, bint equivalent=True) except -2: + cpdef bint isMappingValid(self, Graph other, dict mapping, bint equivalent=True, bint strict=True) except -2: """ Check that a proposed `mapping` of vertices from `self` to `other` is valid by checking that the vertices and edges involved in the - mapping are mutually equivalent. If equivalent is true it checks - if atoms and edges are equivalent, if false it checks if they - are specific cases of each other. + mapping are mutually equivalent. If equivalent is ``True`` it checks + if atoms and edges are equivalent, if ``False`` it checks if they + are specific cases of each other. If strict is ``True``, electrons + and bond orders are considered, and ignored if ``False``. """ cdef Vertex vertex1, vertex2 cdef list vertices1, vertices2 cdef bint selfHasEdge, otherHasEdge cdef int i, j - - method = 'equivalent' if equivalent else 'isSpecificCaseOf' - + # Check that the mapped pairs of vertices compare True for vertex1, vertex2 in mapping.items(): - if not getattr(vertex1,method)(vertex2): - return False + if equivalent: + if not vertex1.equivalent(vertex2, strict=strict): + return False + else: + if not vertex1.isSpecificCaseOf(vertex2): + return False # Check that any edges connected mapped vertices are equivalent vertices1 = mapping.keys() @@ -1104,10 +1107,15 @@ cdef class Graph(object): otherHasEdge = other.hasEdge(vertices2[i], vertices2[j]) if selfHasEdge and otherHasEdge: # Both graphs have the edge, so we must check it for equivalence - edge1 = self.getEdge(vertices1[i], vertices1[j]) - edge2 = other.getEdge(vertices2[i], vertices2[j]) - if not getattr(edge1,method)(edge2): - return False + if strict: + edge1 = self.getEdge(vertices1[i], vertices1[j]) + edge2 = other.getEdge(vertices2[i], vertices2[j]) + if equivalent: + if not edge1.equivalent(edge2): + return False + else: + if not edge1.isSpecificCaseOf(edge2): + return False elif not equivalent and selfHasEdge and not otherHasEdge: #in the subgraph case self can have edges other doesn't have continue From 90529757eb16f408d02bc2b7b950e28c6da02c8b Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 4 Apr 2019 17:49:08 -0400 Subject: [PATCH 16/20] Add strict argument to isIdentical methods Of Molecule, Species, and Reaction, which is passed to Graph.isMappingValid --- rmgpy/molecule/molecule.pxd | 2 +- rmgpy/molecule/molecule.py | 6 ++++-- rmgpy/molecule/moleculeTest.py | 12 ++++++++++++ rmgpy/reaction.py | 2 +- rmgpy/species.pxd | 2 +- rmgpy/species.py | 8 +++++--- 6 files changed, 24 insertions(+), 8 deletions(-) diff --git a/rmgpy/molecule/molecule.pxd b/rmgpy/molecule/molecule.pxd index f76268afeb..70e0d363ab 100644 --- a/rmgpy/molecule/molecule.pxd +++ b/rmgpy/molecule/molecule.pxd @@ -258,7 +258,7 @@ cdef class Molecule(Graph): cpdef bint atomIDValid(self) - cpdef bint isIdentical(self, Molecule other) except -2 + cpdef bint isIdentical(self, Molecule other, bint strict=?) except -2 cpdef dict enumerate_bonds(self) diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index f5deb1cefc..a421ff50d9 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -2312,13 +2312,15 @@ def atomIDValid(self): return True return False - def isIdentical(self, other): + def isIdentical(self, other, strict=True): """ Performs isomorphism checking, with the added constraint that atom IDs must match. Primary use case is tracking atoms in reactions for reaction degeneracy determination. Returns :data:`True` if two graphs are identical and :data:`False` otherwise. + + If ``strict=False``, performs the check ignoring electrons and resonance structures. """ cython.declare(atomIndicies=set, otherIndices=set, atomList=list, otherList=list, mapping = dict) @@ -2340,7 +2342,7 @@ def isIdentical(self, other): for atom1, atom2 in itertools.izip(atomList, otherList): mapping[atom1] = atom2 - return self.isMappingValid(other, mapping) + return self.isMappingValid(other, mapping, equivalent=True, strict=strict) else: # The molecules don't have the same set of indices, so they are not identical return False diff --git a/rmgpy/molecule/moleculeTest.py b/rmgpy/molecule/moleculeTest.py index 8d97f0e513..810e31e964 100644 --- a/rmgpy/molecule/moleculeTest.py +++ b/rmgpy/molecule/moleculeTest.py @@ -2291,6 +2291,18 @@ def testIdenticalTrue(self): self.assertTrue(mol.isIsomorphic(molCopy)) self.assertTrue(mol.isIdentical(molCopy)) + def testIdenticalTrue2(self): + """Test that isIdentical with strict=False returns True with allyl""" + mol = Molecule(SMILES='C=C[CH2]') + mol.assignAtomIDs() + res = mol.generate_resonance_structures(keep_isomorphic=True) + self.assertEqual(len(res), 2) + + mol2 = res[1] + self.assertTrue(mol.isIsomorphic(mol2)) + self.assertFalse(mol.isIdentical(mol2)) + self.assertTrue(mol.isIdentical(mol2, strict=False)) + def testIdenticalFalse(self): """Test that the isIdentical returns False with butane""" mol = Molecule(SMILES='CCCC') diff --git a/rmgpy/reaction.py b/rmgpy/reaction.py index 1b619ec086..d0ad0bbf78 100644 --- a/rmgpy/reaction.py +++ b/rmgpy/reaction.py @@ -1354,7 +1354,7 @@ def same(object1, object2, _check_identical=check_identical, _only_check_label=o if _only_check_label: return str(object1) == str(object2) elif _check_identical: - return object1.isIdentical(object2) + return object1.isIdentical(object2, strict=_strict) else: return object1.isIsomorphic(object2, generateInitialMap=_generate_initial_map, strict=_strict) diff --git a/rmgpy/species.pxd b/rmgpy/species.pxd index 0e126d1281..20532306b0 100644 --- a/rmgpy/species.pxd +++ b/rmgpy/species.pxd @@ -60,7 +60,7 @@ cdef class Species: cpdef bint isIsomorphic(self, other, bint generateInitialMap=?, bint strict=?) except -2 - cpdef bint isIdentical(self, other) except -2 + cpdef bint isIdentical(self, other, bint strict=?) except -2 cpdef bint is_structure_in_list(self, list species_list) except -2 diff --git a/rmgpy/species.py b/rmgpy/species.py index 9911f6afdd..523409df8a 100644 --- a/rmgpy/species.py +++ b/rmgpy/species.py @@ -226,19 +226,21 @@ def isIsomorphic(self, other, generateInitialMap=False, strict=True): raise ValueError('Unexpected value "{0!r}" for other parameter; should be a Molecule or Species object.'.format(other)) return False - def isIdentical(self, other): + def isIdentical(self, other, strict=True): """ Return ``True`` if at least one molecule of the species is identical to `other`, which can be either a :class:`Molecule` object or a :class:`Species` object. + + If ``strict=False``, performs the check ignoring electrons and resonance structures. """ if isinstance(other, Molecule): for molecule in self.molecule: - if molecule.isIdentical(other): + if molecule.isIdentical(other, strict=strict): return True elif isinstance(other, Species): for molecule1 in self.molecule: for molecule2 in other.molecule: - if molecule1.isIdentical(molecule2): + if molecule1.isIdentical(molecule2, strict=strict): return True else: raise ValueError('Unexpected value "{0!r}" for other parameter;' From 78f8a2dd0c25981eca64f360697f729830c87363 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 4 Apr 2019 14:59:17 -0400 Subject: [PATCH 17/20] Do not generate resonance structures for degeneracy determination Instead, use the strict=False option to ignore resonance issues Change ensure_species to not generate resonance structures by default Add call to ensure_independent_atom_ids in addReverseAttribute because the changes to resonance structure generation means that products may not necessarily have proper atom IDs --- rmgpy/data/kinetics/common.py | 5 ++--- rmgpy/data/kinetics/family.py | 4 +++- rmgpy/reaction.py | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/rmgpy/data/kinetics/common.py b/rmgpy/data/kinetics/common.py index c745e492da..1f2377e2c9 100644 --- a/rmgpy/data/kinetics/common.py +++ b/rmgpy/data/kinetics/common.py @@ -275,7 +275,6 @@ def find_degenerate_reactions(rxn_list, same_reactants=None, template=None, kine # with degenerate transition states sorted_rxns = [] for rxn0 in selected_rxns: - # find resonance structures for rxn0 rxn0.ensure_species() if len(sorted_rxns) == 0: # This is the first reaction, so create a new sublist @@ -288,9 +287,9 @@ def find_degenerate_reactions(rxn_list, same_reactants=None, template=None, kine identical = False sameTemplate = True for rxn in sub_list: - isomorphic = rxn0.isIsomorphic(rxn, checkIdentical=False, checkTemplateRxnProducts=True) + isomorphic = rxn0.isIsomorphic(rxn, checkIdentical=False, strict=False, checkTemplateRxnProducts=True) if isomorphic: - identical = rxn0.isIsomorphic(rxn, checkIdentical=True, checkTemplateRxnProducts=True) + identical = rxn0.isIsomorphic(rxn, checkIdentical=True, strict=False, checkTemplateRxnProducts=True) if identical: # An exact copy of rxn0 is already in our list, so we can move on break diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index e08c386043..b511493143 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -1679,6 +1679,8 @@ def addReverseAttribute(self, rxn, react_non_reactive=True): elif rxn.products[1].isIsomorphic(rxn.products[2]): sameReactants = 2 + ensure_independent_atom_ids(rxn.products) + reactionList = self.__generateReactions([spc.molecule for spc in rxn.products], products=rxn.reactants, forward=True, react_non_reactive=react_non_reactive) @@ -1713,7 +1715,7 @@ def addReverseAttribute(self, rxn, react_non_reactive=True): else: logging.error("Still experiencing error: Expecting one matching reverse reaction, not {0} in reaction family {1} for forward reaction {2}.\n".format(len(reactions), self.label, str(rxn))) raise KineticsError("Did not find reverse reaction in reaction family {0} for reaction {1}.".format(self.label, str(rxn))) - elif len(reactions) > 1 and not all([reactions[0].isIsomorphic(other, checkTemplateRxnProducts=True) for other in reactions]): + elif len(reactions) > 1 and not all([reactions[0].isIsomorphic(other, strict=False, checkTemplateRxnProducts=True) for other in reactions]): logging.error("Expecting one matching reverse reaction. Recieved {0} reactions with multiple non-isomorphic ones in reaction family {1} for forward reaction {2}.\n".format(len(reactions), self.label, str(rxn))) logging.info("Found the following reverse reactions") for rxn0 in reactions: diff --git a/rmgpy/reaction.py b/rmgpy/reaction.py index d0ad0bbf78..491f4f7824 100644 --- a/rmgpy/reaction.py +++ b/rmgpy/reaction.py @@ -1186,7 +1186,7 @@ def copy(self): return other - def ensure_species(self, reactant_resonance=False, product_resonance=True): + def ensure_species(self, reactant_resonance=False, product_resonance=False): """ Ensure the reaction contains species objects in its reactant and product attributes. If the reaction is found to hold molecule objects, it From 4084b1ed864975b92d5f2adb99282ccf11c6efd3 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Tue, 30 Oct 2018 14:04:46 -0400 Subject: [PATCH 18/20] Make sure selected molecule is reactive --- rmgpy/data/kinetics/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rmgpy/data/kinetics/common.py b/rmgpy/data/kinetics/common.py index 1f2377e2c9..94961af090 100644 --- a/rmgpy/data/kinetics/common.py +++ b/rmgpy/data/kinetics/common.py @@ -214,7 +214,7 @@ def independent_ids(): logging.debug('identical atom ids found between species. regenerating') for species in input_species: unreactive_mol_list = [mol for mol in species.molecule if not mol.reactive] - mol = species.molecule[0] + mol = [mol for mol in species.molecule if mol.reactive][0] # Choose first reactive molecule mol.assignAtomIDs() species.molecule = [mol] # Remake resonance structures with new labels From 27baa4e35dbf9a33c6c7ec23a84a6e33dc287e79 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 4 Apr 2019 16:23:42 -0400 Subject: [PATCH 19/20] Enable Species instantiation by SMILES or InChI argument Add SMILES property and unit tests --- rmgpy/species.pxd | 1 + rmgpy/species.py | 35 ++++++++++++++---- rmgpy/speciesTest.py | 87 ++++++++++++++++++-------------------------- 3 files changed, 65 insertions(+), 58 deletions(-) diff --git a/rmgpy/species.pxd b/rmgpy/species.pxd index 20532306b0..5611ee4d72 100644 --- a/rmgpy/species.pxd +++ b/rmgpy/species.pxd @@ -55,6 +55,7 @@ cdef class Species: cdef public bint explicitlyAllowed cdef str _fingerprint cdef str _inchi + cdef str _smiles cpdef generate_resonance_structures(self, bint keep_isomorphic=?, bint filter_structures=?) diff --git a/rmgpy/species.py b/rmgpy/species.py index 523409df8a..c9239236fe 100644 --- a/rmgpy/species.py +++ b/rmgpy/species.py @@ -81,16 +81,16 @@ class Species(object): always considered regardless of this variable `props` A generic 'properties' dictionary to store user-defined flags `aug_inchi` Unique augmented inchi - `isSolvent` Boolean describing whether this species is the solvent + `symmetryNumber` Estimated symmetry number of the species, using the resonance hybrid `creationIteration` Iteration which the species is created within the reaction mechanism generation algorithm + `explicitlyAllowed` Flag to exempt species from forbidden structure checks ======================= ==================================================== """ - def __init__(self, index=-1, label='', thermo=None, conformer=None, - molecule=None, transportData=None, molecularWeight=None, - energyTransferModel=None, reactive=True, props=None, aug_inchi=None, - symmetryNumber = -1, creationIteration = 0, explicitlyAllowed=False): + def __init__(self, index=-1, label='', thermo=None, conformer=None, molecule=None, transportData=None, + molecularWeight=None, energyTransferModel=None, reactive=True, props=None, SMILES='', InChI='', + aug_inchi=None, symmetryNumber = -1, creationIteration = 0, explicitlyAllowed=False): self.index = index self.label = label self.thermo = thermo @@ -108,14 +108,23 @@ def __init__(self, index=-1, label='', thermo=None, conformer=None, self.explicitlyAllowed = explicitlyAllowed self._fingerprint = None self._inchi = None + self._smiles = None + + if InChI and SMILES: + logging.warning('Both InChI and SMILES provided for Species instantiation, using InChI and ignoring SMILES.') + if InChI: + self.molecule = [Molecule(InChI=InChI)] + self._inchi = InChI + elif SMILES: + self.molecule = [Molecule(SMILES=SMILES)] + self._smiles = SMILES + # Check multiplicity of each molecule is the same if molecule is not None and len(molecule)>1: mult = molecule[0].multiplicity for m in molecule[1:]: if mult != m.multiplicity: raise SpeciesError('Multiplicities of molecules in species {species} do not match.'.format(species=label)) - - def __repr__(self): """ @@ -172,6 +181,18 @@ def InChI(self): self._inchi = self.molecule[0].InChI return self._inchi + @property + def SMILES(self): + """ + SMILES string representation of this species. Read-only. + + Note that SMILES representations for different resonance structures of the same species may be different. + """ + if self._smiles is None: + if self.molecule: + self._smiles = self.molecule[0].SMILES + return self._smiles + @property def multiplicity(self): """Fingerprint of this species, taken from molecule attribute. Read-only.""" diff --git a/rmgpy/speciesTest.py b/rmgpy/speciesTest.py index ec8d507e7d..4a10930921 100644 --- a/rmgpy/speciesTest.py +++ b/rmgpy/speciesTest.py @@ -77,7 +77,23 @@ def setUp(self): molecularWeight=(28.03,'amu'), reactive=True, ) - + + self.species2 = Species().fromAdjacencyList( + """ + 1 C u0 p0 c0 {2,D} {6,S} {7,S} + 2 C u0 p0 c0 {1,D} {3,S} {8,S} + 3 C u0 p0 c0 {2,S} {4,D} {9,S} + 4 C u0 p0 c0 {3,D} {5,S} {10,S} + 5 C u0 p0 c0 {4,S} {6,D} {11,S} + 6 C u0 p0 c0 {1,S} {5,D} {12,S} + 7 H u0 p0 c0 {1,S} + 8 H u0 p0 c0 {2,S} + 9 H u0 p0 c0 {3,S} + 10 H u0 p0 c0 {4,S} + 11 H u0 p0 c0 {5,S} + 12 H u0 p0 c0 {6,S} + """) + def testPickle(self): """ Test that a Species object can be pickled and unpickled. @@ -324,63 +340,32 @@ def testGetTransportData(self): def test_fingerprint_property(self): """Test that the fingerprint property works""" - spc = Species().fromAdjacencyList( - """ - 1 C u0 p0 c0 {2,D} {6,S} {7,S} - 2 C u0 p0 c0 {1,D} {3,S} {8,S} - 3 C u0 p0 c0 {2,S} {4,D} {9,S} - 4 C u0 p0 c0 {3,D} {5,S} {10,S} - 5 C u0 p0 c0 {4,S} {6,D} {11,S} - 6 C u0 p0 c0 {1,S} {5,D} {12,S} - 7 H u0 p0 c0 {1,S} - 8 H u0 p0 c0 {2,S} - 9 H u0 p0 c0 {3,S} - 10 H u0 p0 c0 {4,S} - 11 H u0 p0 c0 {5,S} - 12 H u0 p0 c0 {6,S} - """) - - self.assertEqual(spc.fingerprint, 'C6H6') + self.assertEqual(self.species2.fingerprint, 'C6H6') def test_inchi_property(self): """Test that the InChI property works""" - spc = Species().fromAdjacencyList( - """ - 1 C u0 p0 c0 {2,D} {6,S} {7,S} - 2 C u0 p0 c0 {1,D} {3,S} {8,S} - 3 C u0 p0 c0 {2,S} {4,D} {9,S} - 4 C u0 p0 c0 {3,D} {5,S} {10,S} - 5 C u0 p0 c0 {4,S} {6,D} {11,S} - 6 C u0 p0 c0 {1,S} {5,D} {12,S} - 7 H u0 p0 c0 {1,S} - 8 H u0 p0 c0 {2,S} - 9 H u0 p0 c0 {3,S} - 10 H u0 p0 c0 {4,S} - 11 H u0 p0 c0 {5,S} - 12 H u0 p0 c0 {6,S} - """) - - self.assertEqual(spc.InChI, 'InChI=1S/C6H6/c1-2-4-6-5-3-1/h1-6H') + self.assertEqual(self.species2.InChI, 'InChI=1S/C6H6/c1-2-4-6-5-3-1/h1-6H') def test_multiplicity_property(self): """Test that the fingerprint property works""" - spc = Species().fromAdjacencyList( - """ - 1 C u0 p0 c0 {2,D} {6,S} {7,S} - 2 C u0 p0 c0 {1,D} {3,S} {8,S} - 3 C u0 p0 c0 {2,S} {4,D} {9,S} - 4 C u0 p0 c0 {3,D} {5,S} {10,S} - 5 C u0 p0 c0 {4,S} {6,D} {11,S} - 6 C u0 p0 c0 {1,S} {5,D} {12,S} - 7 H u0 p0 c0 {1,S} - 8 H u0 p0 c0 {2,S} - 9 H u0 p0 c0 {3,S} - 10 H u0 p0 c0 {4,S} - 11 H u0 p0 c0 {5,S} - 12 H u0 p0 c0 {6,S} - """) + self.assertEqual(self.species2.multiplicity, 1) + + def test_smiles_property(self): + """Test that the InChI property works""" + self.assertEqual(self.species2.SMILES, 'C1=CC=CC=C1') + + def test_inchi_instantiation(self): + """Test that we can create a species using the InChI argument""" + test = Species(InChI='InChI=1S/C6H6/c1-2-4-6-5-3-1/h1-6H') + + self.assertTrue(test.isIsomorphic(self.species2)) + + def test_smiles_instantiation(self): + """Test that we can create a species using the SMILES argument""" + test = Species(SMILES='C1=CC=CC=C1') + + self.assertTrue(test.isIsomorphic(self.species2)) - self.assertEqual(spc.multiplicity, 1) ################################################################################ From b3ff5c51dc20661b3673a32eae25bb4295017963 Mon Sep 17 00:00:00 2001 From: Max Liu Date: Thu, 4 Apr 2019 17:44:05 -0400 Subject: [PATCH 20/20] Fix reaction degeneracy bug with keep_isomorphic argument In certain situations, we would call ensure_species with keep_isomorphic=False, which would ultimately result in wrong degeneracy values. This fixes that oversight and makes sure that keep_isomorphic=True when generating reactions. A unit test is also added. --- rmgpy/data/kinetics/common.py | 2 +- rmgpy/data/kinetics/kineticsTest.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/rmgpy/data/kinetics/common.py b/rmgpy/data/kinetics/common.py index 94961af090..c4bebaa21e 100644 --- a/rmgpy/data/kinetics/common.py +++ b/rmgpy/data/kinetics/common.py @@ -198,7 +198,7 @@ def ensure_independent_atom_ids(input_species, resonance=True): Modifies the list in place (replacing :class:`Molecule` with :class:`Species`). Returns None. """ - ensure_species(input_species, resonance=resonance) + ensure_species(input_species, resonance=resonance, keep_isomorphic=True) # Method to check that all species' atom ids are different def independent_ids(): num_atoms = 0 diff --git a/rmgpy/data/kinetics/kineticsTest.py b/rmgpy/data/kinetics/kineticsTest.py index f6b2df1ab5..58d3f3322c 100644 --- a/rmgpy/data/kinetics/kineticsTest.py +++ b/rmgpy/data/kinetics/kineticsTest.py @@ -590,6 +590,19 @@ def test_degeneracy_multiple_resonance_different_template(self): self.assertFalse(reaction_list[0].duplicate) + def test_degeneracy_resonance_keep_isomorphic(self): + """Test that we get the correct degeneracy for [CH2]C=C[CH2] + [H]. + + Incorrect results would be obtained if isomorphic resonance structures are not kept.""" + family_label = 'R_Recombination' + reactants = ['[CH2]C=C[CH2]', '[OH]'] + products = ['[CH2]C(O)C=C'] + + correct_rxn_num = 1 + correct_degeneracy = {2} + + self.assert_correct_reaction_degeneracy(reactants, correct_rxn_num, correct_degeneracy, family_label, products) + class TestKineticsCommentsParsing(unittest.TestCase):