Merge pull request #1538 from ReactionMechanismGenerator/catemp1

Non-cat changes to master from cat branch, 1

rmgpy/chemkin.pyx

@@ -1649,28 +1649,30 @@ def writeKineticsEntry(reaction, speciesList, verbose = True, javaLibrary = Fals
 
     if isinstance(kinetics, _kinetics.Arrhenius):
         string += '{0:<9.6e} {1:<9.3f} {2:<9.3f}'.format(
-            kinetics.A.value_si/ (kinetics.T0.value_si ** kinetics.n.value_si) * 1.0e6 ** (numReactants - 1),
+            kinetics.A.value_si / (kinetics.T0.value_si ** kinetics.n.value_si) * kinetics.A.getConversionFactorFromSItoCmMolS(),
             kinetics.n.value_si,
             kinetics.Ea.value_si / 4184.
         )
     elif isinstance(kinetics, (_kinetics.Lindemann, _kinetics.Troe)):
         arrhenius = kinetics.arrheniusHigh
         string += '{0:<9.3e} {1:<9.3f} {2:<9.3f}'.format(
-            arrhenius.A.value_si / (arrhenius.T0.value_si ** arrhenius.n.value_si) * 1.0e6 ** (numReactants - 1),
+            arrhenius.A.value_si / (arrhenius.T0.value_si ** arrhenius.n.value_si) * arrhenius.A.getConversionFactorFromSItoCmMolS(),
             arrhenius.n.value_si,
             arrhenius.Ea.value_si / 4184.
         )
     elif isinstance(kinetics, _kinetics.ThirdBody):
         arrhenius = kinetics.arrheniusLow
+        assert 0.999 < arrhenius.A.getConversionFactorFromSItoCmMolS() / 1.0e6 ** (numReactants) < 1.001  # debugging; for gas phase only
         string += '{0:<9.3e} {1:<9.3f} {2:<9.3f}'.format(
-            arrhenius.A.value_si / (arrhenius.T0.value_si ** arrhenius.n.value_si) * 1.0e6 ** (numReactants),
+            arrhenius.A.value_si / (arrhenius.T0.value_si ** arrhenius.n.value_si) * arrhenius.A.getConversionFactorFromSItoCmMolS(),
             arrhenius.n.value_si,
             arrhenius.Ea.value_si / 4184.
         )
     elif hasattr(kinetics,'highPlimit') and kinetics.highPlimit is not None:
         arrhenius = kinetics.highPlimit
+        assert 0.999 < arrhenius.A.getConversionFactorFromSItoCmMolS() / 1.0e6 ** (numReactants - 1) < 1.001  # debugging; for gas phase only
         string += '{0:<9.3e} {1:<9.3f} {2:<9.3f}'.format(
-            arrhenius.A.value_si / (arrhenius.T0.value_si ** arrhenius.n.value_si) * 1.0e6 ** (numReactants - 1),
+            arrhenius.A.value_si / (arrhenius.T0.value_si ** arrhenius.n.value_si) * arrhenius.A.getConversionFactorFromSItoCmMolS(),
             arrhenius.n.value_si,
             arrhenius.Ea.value_si / 4184.
             )
@@ -1699,8 +1701,9 @@ def writeKineticsEntry(reaction, speciesList, verbose = True, javaLibrary = Fals
         if isinstance(kinetics, (_kinetics.Lindemann, _kinetics.Troe)):
             # Write low-P kinetics
             arrhenius = kinetics.arrheniusLow
+            assert 0.999 < arrhenius.A.getConversionFactorFromSItoCmMolS() / 1.0e6 ** (numReactants) < 1.001  # debugging; for gas phase only
             string += '    LOW/ {0:<9.3e} {1:<9.3f} {2:<9.3f}/\n'.format(
-                arrhenius.A.value_si / (arrhenius.T0.value_si ** arrhenius.n.value_si) * 1.0e6 ** (numReactants),
+                arrhenius.A.value_si / (arrhenius.T0.value_si ** arrhenius.n.value_si) * arrhenius.A.getConversionFactorFromSItoCmMolS(),
                 arrhenius.n.value_si,
                 arrhenius.Ea.value_si / 4184.
             )
@@ -1714,14 +1717,16 @@ def writeKineticsEntry(reaction, speciesList, verbose = True, javaLibrary = Fals
         for P, arrhenius in zip(kinetics.pressures.value_si, kinetics.arrhenius):
             if isinstance(arrhenius, _kinetics.MultiArrhenius):
                 for arrh in arrhenius.arrhenius:
+                    assert 0.999 < arrh.A.getConversionFactorFromSItoCmMolS() / 1.0e6 ** (numReactants - 1) < 1.001  # debugging; for gas phase only
                     string += '    PLOG/ {0:<9.6f} {1:<9.3e} {2:<9.3f} {3:<9.3f}/\n'.format(P / 101325.,
-                    arrh.A.value_si / (arrh.T0.value_si ** arrh.n.value_si) * 1.0e6 ** (numReactants - 1),
+                    arrh.A.value_si / (arrh.T0.value_si ** arrh.n.value_si) * arrh.A.getConversionFactorFromSItoCmMolS(),
                     arrh.n.value_si,
                     arrh.Ea.value_si / 4184.
                     )
             else:
-                string += '    PLOG/ {0:<9.3f} {1:<9.3e} {2:<9.3f} {3:<9.3f}/\n'.format(P / 101325.,
-                    arrhenius.A.value_si / (arrhenius.T0.value_si ** arrhenius.n.value_si) * 1.0e6 ** (numReactants - 1),
+                assert 0.999 < arrhenius.A.getConversionFactorFromSItoCmMolS() / 1.0e6 ** (numReactants - 1) < 1.001  # debugging; for gas phase only
+                string += '    PLOG/ {0:<9.6f} {1:<9.3e} {2:<9.3f} {3:<9.3f}/\n'.format(P / 101325.,
+                    arrhenius.A.value_si / (arrhenius.T0.value_si ** arrhenius.n.value_si) * arrhenius.A.getConversionFactorFromSItoCmMolS(),
                     arrhenius.n.value_si,
                     arrhenius.Ea.value_si / 4184.
                 )
@@ -1742,7 +1747,7 @@ def writeKineticsEntry(reaction, speciesList, verbose = True, javaLibrary = Fals
             for i in range(kinetics.degreeT):
                 for j in range(kinetics.degreeP):
                     coeffs.append(kinetics.coeffs.value_si[i,j])
-            coeffs[0] += 6 * (numReactants - 1)
+            coeffs[0] += 6 * (numReactants - 1)  # bypassing the Units.getConversionFactorFromSItoCmMolS() because it's in log10 space?
             for i in range(len(coeffs)):
                 if i % 5 == 0: string += '    CHEB/'
                 string += ' {0:<12.3e}'.format(coeffs[i])
@@ -1990,31 +1995,23 @@ def saveChemkin(reactionModel, path, verbose_path, dictionaryPath=None, transpor
     """
     Save a Chemkin file for the current model as well as any desired output
     species and reactions to `path`. If `saveEdgeSpecies` is True, then 
-    a chemkin file and dictionary file for the core and edge species and reactions
-    will be saved.  
+    a chemkin file and dictionary file for the core AND edge species and reactions
+    will be saved.  It also saves verbose versions of each file.
     """
-
-    if saveEdgeSpecies == False:
-        speciesList = reactionModel.core.species + reactionModel.outputSpeciesList
-        rxnList = reactionModel.core.reactions + reactionModel.outputReactionList
-        saveChemkinFile(path, speciesList, rxnList, verbose = False, checkForDuplicates=False) # We should already have marked everything as duplicates by now        
-        logging.info('Saving current model to verbose Chemkin file...')
-        saveChemkinFile(verbose_path, speciesList, rxnList, verbose = True, checkForDuplicates=False)
-        if dictionaryPath:
-            saveSpeciesDictionary(dictionaryPath, speciesList)
-        if transportPath:
-            saveTransportFile(transportPath, speciesList)
-
-    else:
+    if saveEdgeSpecies:
         speciesList = reactionModel.core.species + reactionModel.edge.species
         rxnList = reactionModel.core.reactions + reactionModel.edge.reactions
-        saveChemkinFile(path, speciesList, rxnList, verbose = False, checkForDuplicates=False)        
-        logging.info('Saving current core and edge to verbose Chemkin file...')
-        saveChemkinFile(verbose_path, speciesList, rxnList, verbose = True, checkForDuplicates=False)
-        if dictionaryPath:
-            saveSpeciesDictionary(dictionaryPath, speciesList)
-        if transportPath:
-            saveTransportFile(transportPath, speciesList)
+    else:
+        speciesList = reactionModel.core.species + reactionModel.outputSpeciesList
+        rxnList = reactionModel.core.reactions + reactionModel.outputReactionList
+
+    saveChemkinFile(path, speciesList, rxnList, verbose = False, checkForDuplicates=False) # We should already have marked everything as duplicates by now
+    logging.info('Saving verbose version of Chemkin file...')
+    saveChemkinFile(verbose_path, speciesList, rxnList, verbose=True, checkForDuplicates=False)
+    if dictionaryPath:
+        saveSpeciesDictionary(dictionaryPath, speciesList)
+    if transportPath:
+        saveTransportFile(transportPath, speciesList)
 
 def saveChemkinFiles(rmg):
     """

rmgpy/data/kinetics/database.py

@@ -207,7 +207,11 @@ def loadFamilies(self, path, families=None, depositories=None):
         for label in selected_families:
             familyPath = os.path.join(path, label)
             family = KineticsFamily(label=label)
-            family.load(familyPath, self.local_context, self.global_context, depositoryLabels=depositories)
+            try:
+                family.load(familyPath, self.local_context, self.global_context, depositoryLabels=depositories)
+            except:
+                logging.error("Error when loading reaction family {!r}".format(familyPath))
+                raise
             self.families[label] = family
 
     def loadLibraries(self, path, libraries=None):
@@ -532,7 +536,12 @@ def react_molecules(self, molecules, products=None, only_families=None, prod_res
         reaction_list = []
         for label, family in self.families.iteritems():
             if only_families is None or label in only_families:
-                reaction_list.extend(family.generateReactions(molecules, products=products, prod_resonance=prod_resonance))
+                try:
+                    reaction_list.extend(family.generateReactions(molecules, products=products, prod_resonance=prod_resonance))
+                except:
+                    logging.error("Problem family: {}".format(label))
+                    logging.error("Problem reactants: {}".format(molecules))
+                    raise
 
         for reactant in molecules:
             reactant.clearLabeledAtoms()

rmgpy/data/thermo.py

@@ -1998,7 +1998,7 @@ def __addGroupThermoData(self, thermoData, database, molecule, atom):
         """
         node0 = database.descendTree(molecule, atom, None)
         if node0 is None:
-            raise KeyError('Node not found in database.')
+            raise KeyError('Node not found in thermo database for atom {0} in molecule {1}.'.format(atom, molecule))
 
         # It's possible (and allowed) that items in the tree may not be in the
         # library, in which case we need to fall up the tree until we find an

rmgpy/molecule/atomtype.py

@@ -224,8 +224,8 @@ def getFeatures(self):
 The atomTypes naming convention is:
 <element> <valence> <characteristic bonds> <charge(optional)>
 For example:
-- N3d is nitrogen with valence=3 (i.e., 3 electronce are able to form bonds or remain as radicals) with one double bond
-- S2tc is a charged sulful with valence=2 with a triple bonds
+- N3d is nitrogen with valence=3 (i.e., 3 electrons are able to form bonds or remain as radicals) with one double bond
+- S2tc is a charged sulfur with valence=2 with a triple bonds
 - Oa is atomic oxygen, i.e., a closed shell atom
 Some charged atom types were merged together, and are marked as '*Composite atomType'
 """
@@ -626,9 +626,9 @@ def getFeatures(self):
 atomTypes['F'  ].setActions(incrementBond=[],               decrementBond=[],               formBond=['F'],        breakBond=['F'],        incrementRadical=['F'],   decrementRadical=['F'],   incrementLonePair=[],      decrementLonePair=[])
 atomTypes['F1s'].setActions(incrementBond=[],               decrementBond=[],               formBond=['F1s'],      breakBond=['F1s'],      incrementRadical=['F1s'], decrementRadical=['F1s'], incrementLonePair=[],      decrementLonePair=[])
 
-#list of elements that do not have more specific atomTypes
 #these are ordered on priority of picking if we encounter a more general atomType for make
 allElements=['H', 'C', 'O', 'N', 'S', 'Si', 'Cl', 'Ne', 'Ar', 'He',]
+#list of elements that do not have more specific atomTypes
 nonSpecifics=['H', 'He', 'Ne', 'Ar',]
 
 for atomType in atomTypes.values():

rmgpy/molecule/draw.py

@@ -307,27 +307,31 @@ def __generateCoordinates(self):
         """
         atoms = self.molecule.atoms
         Natoms = len(atoms)
-        flag_charge = 0
 
-        for atom in self.molecule.atoms:
-            if atom.charge != 0:
-                 flag_charge = 1
-                 break
+
 
         # Initialize array of coordinates
         self.coordinates = coordinates = numpy.zeros((Natoms, 2))
-
-        if flag_charge == 1:
-            # If there are only one or two atoms to draw, then determining the
-            # coordinates is trivial
-            if Natoms == 1:
-                self.coordinates[0,:] = [0.0, 0.0]
-                return self.coordinates
-            elif Natoms == 2:
-                self.coordinates[0,:] = [-0.5, 0.0]
-                self.coordinates[1,:] = [0.5, 0.0]
-                return self.coordinates
-
+
+        # If there are only one or two atoms to draw, then determining the
+        # coordinates is trivial
+        if Natoms == 1:
+            self.coordinates[0, :] = [0.0, 0.0]
+            return self.coordinates
+        elif Natoms == 2:
+            self.coordinates[0, :] = [-0.5, 0.0]
+            self.coordinates[1, :] = [0.5, 0.0]
+            return self.coordinates
+
+        # Decide whether we can use RDKit or have to generate coordinates ourselves
+        for atom in self.molecule.atoms:
+            if atom.charge != 0:
+                 useRDKit = False
+                 break
+        else: # didn't break
+            useRDKit = True
+
+        if not useRDKit:
             if len(self.cycles) > 0:
                 # Cyclic molecule
                 backbone = self.__findCyclicBackbone()
@@ -350,7 +354,8 @@ def __generateCoordinates(self):
                 else:
                     angle = math.atan2(vector0[0], vector0[1]) - math.pi / 2
                     rot = numpy.array([[math.cos(angle), math.sin(angle)], [-math.sin(angle), math.cos(angle)]], numpy.float64)
-                    coordinates = numpy.dot(coordinates, rot)
+                    # need to keep self.coordinates and coordinates referring to the same object
+                    self.coordinates = coordinates = numpy.dot(coordinates, rot)
 
             # Center backbone at origin
             xmin = numpy.min(coordinates[:,0])
@@ -375,8 +380,7 @@ def __generateCoordinates(self):
             return coordinates
 
         else:
-
-            # Use rdkit 2D coordinate generation:
+            # Use RDKit 2D coordinate generation:
 
             # Generate the RDkit molecule from the RDkit molecule, use geometry
             # in order to match the atoms in the rdmol with the atoms in the
@@ -390,7 +394,7 @@ def __generateCoordinates(self):
             for atom in atoms:
                 index = rdAtomIdx[atom]
                 point = rdmol.GetConformer(0).GetAtomPosition(index)
-                coordinates[index,:]= [point.x*0.6, point.y*0.6]
+                coordinates[index,:] = [point.x*0.6, point.y*0.6]
 
             # RDKit generates some molecules more vertically than horizontally,
             # Especially linear ones. This will reflect any molecule taller than

rmgpy/molecule/graphTest.py

@@ -427,6 +427,47 @@ def test_isomorphism(self):
         self.assertTrue(graph2.isIsomorphic(graph1))
         self.assertTrue(graph2.isSubgraphIsomorphic(graph1))
 
+
+    def test_isomorphism_disconnected(self):
+        """
+        Check the graph isomorphism for broken graphs.
+        
+        This tries to match graphs with a missing bond, 
+        eg. [ 0-1-2-3-4  5 ] should match [ 0-1-2-3-4  5 ]
+        """
+
+        vertices1 = [Vertex() for i in range(6)]
+        edges1 = [
+            Edge(vertices1[0], vertices1[1]),
+            Edge(vertices1[1], vertices1[2]),
+            Edge(vertices1[2], vertices1[3]),
+            Edge(vertices1[3], vertices1[4]),
+            #Edge(vertices1[4], vertices1[5]),
+        ]
+
+        vertices2 = [Vertex() for i in range(6)]
+        edges2 = [
+            Edge(vertices2[0], vertices2[1]),
+            Edge(vertices2[1], vertices2[2]),
+            Edge(vertices2[2], vertices2[3]),
+            Edge(vertices2[3], vertices2[4]),
+            #Edge(vertices2[4], vertices2[5]),
+        ]
+
+        graph1 = Graph()
+        for vertex in vertices1: graph1.addVertex(vertex)
+        for edge in edges1: graph1.addEdge(edge)
+
+        graph2 = Graph()
+        for vertex in vertices2: graph2.addVertex(vertex)
+        for edge in edges2: graph2.addEdge(edge)
+
+        self.assertTrue(graph1.isIsomorphic(graph2))
+        self.assertTrue(graph1.isSubgraphIsomorphic(graph2))
+        self.assertTrue(graph2.isIsomorphic(graph1))
+        self.assertTrue(graph2.isSubgraphIsomorphic(graph1))
+        self.assertTrue(len(graph1.findSubgraphIsomorphisms(graph2)) > 0)
+
     def test_subgraphIsomorphism(self):
         """
         Check the subgraph isomorphism functions.

rmgpy/molecule/vf2.pyx

@@ -27,7 +27,7 @@
 
 """
 This module contains graph ismorphism functions that implement the VF2
-algorithm of Vento and Foggia.
+algorithm of Vento and Foggia.  http://dx.doi.org/10.1109/TPAMI.2004.75
 """
 
 cimport cython
@@ -203,6 +203,17 @@ cdef class VF2:
             return True
 
         # Create list of pairs of candidates for inclusion in mapping
+        """
+        10.1109/TPAMI.2004.75 says:
+        "The set P(s) will be made of all the node pairs (n,m),
+        with n belonging to T1out(s) and m to T2out(s),
+        unless one of these two sets is empty. In this case,
+        the set P(s) is likewise obtained by considering
+        T1in(s) and T2in(s), respectively."
+
+        But: for us, bonds are not directional, so ignore Tin(s)
+        and just use Tout(s) which is what we call "terminals".
+        """
         hasTerminals = False
         for vertex2 in self.graph2.vertices:
             if vertex2.ignore:
@@ -212,14 +223,32 @@ cdef class VF2:
                 hasTerminals = True
                 break
         else:
-            vertex2 = self.graph2.vertices[0]
-
+            """
+            "In presence of not connected graphs, for some state s,
+            all of the above sets may be empty. In this case,
+            the set of candidate pairs making up P(s) will be
+            the set Pd(s) of all the pairs of nodes not contained
+            neither in G1(s) nor in G2(s)."
+
+            So: use nodes not yet mapped.
+            """
+            # Take first unmapped vertex
+            for vertex2 in self.graph2.vertices:
+                if vertex2.mapping is None:
+                    break
+            else:
+                raise VF2Error("Still seeking candidate pairs but all nodes in graph2 are already mapped.")
+
         for vertex1 in self.graph1.vertices:
             if vertex1.ignore:
                 continue
             # If terminals are available, then skip vertices in the first
             # graph that are not terminals
-            if hasTerminals and not vertex1.terminal: continue
+            if hasTerminals and not vertex1.terminal:
+                continue
+            # Otherwise take any node that is not already matched
+            if vertex1.mapping is not None:
+                continue
             # Propose a pairing
             if self.feasible(vertex1, vertex2):
                 # Add proposed match to mapping
@@ -230,7 +259,7 @@ cdef class VF2:
                     return True
                 # Undo proposed match
                 self.removeFromMapping(vertex1, vertex2)
-                
+
         # None of the proposed matches led to a complete isomorphism, so return False
         return False     
 

rmgpy/quantity.pxd

@@ -38,6 +38,8 @@ cdef class Units(object):
     cpdef double getConversionFactorToSI(self) except -1
 
     cpdef double getConversionFactorFromSI(self) except -1
+
+    cpdef double getConversionFactorFromSItoCmMolS(self) except -1
 
 ################################################################################