Skip to content

Commit

Permalink
Merge pull request #468 from ReactionMechanismGenerator/fixGroupsAlgo…
Browse files Browse the repository at this point in the history
…rithm

Fix group algorithm that was breaking database tests.  
When matching a structure against a functional group, labeled atoms in the structure but not the group are to be ignored. Previously we temporarily removed the atoms before doing the graph isomorphism. This now causes problems with the sort method which is used at the start of the isomorphism check. So now we just mark the atoms that should be ignored, and ignore them in the VF2 subgraph algorithm.  Seems to work (or at least passes the unit tests).
  • Loading branch information
rwest committed Oct 18, 2015
2 parents 7029295 + 7c5cde8 commit 21b83e3
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 9 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ install:

script:
- make test
- make test-database
- make eg1
- make eg4
- make eg5
Expand Down
17 changes: 8 additions & 9 deletions rmgpy/data/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -948,18 +948,17 @@ def matchNodeToStructure(self, node, structure, atoms, strict=False):
# Passed semantic checks, so add to maps of already-matched atoms
initialMap[atom] = center
# Labeled atoms in the structure that are not in the group should
# not be considered in the isomorphism check, so remove them temporarily
# not be considered in the isomorphism check, so flag them temporarily
# Without this we would hit a lot of nodes that are ambiguous
removedAtoms = []
for label, atom in structure.getLabeledAtoms().iteritems():
if label not in centers:
removedAtoms.append(atom)
structure.atoms.remove(atom)
flaggedAtoms = [atom for label, atom in structure.getLabeledAtoms().iteritems() if label not in centers]
for atom in flaggedAtoms: atom.ignore = True

# use mapped (labeled) atoms to try to match subgraph
result = structure.isSubgraphIsomorphic(group, initialMap)
# Restore atoms removed in previous step
for atom in removedAtoms:
structure.atoms.append(atom)

# Restore atoms flagged in previous step
for atom in flaggedAtoms: atom.ignore = False

return result

def descendTree(self, structure, atoms, root=None, strict=False):
Expand Down
1 change: 1 addition & 0 deletions rmgpy/molecule/graph.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ cdef class Vertex(object):
cdef public short sortingLabel
cdef public bint terminal
cdef public Vertex mapping
cdef public bint ignore

cpdef Vertex copy(self)

Expand Down
1 change: 1 addition & 0 deletions rmgpy/molecule/graph.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ cdef class Vertex(object):
def __init__(self):
self.edges = {}
self.resetConnectivityValues()
self.ignore = False

def __reduce__(self):
"""
Expand Down
6 changes: 6 additions & 0 deletions rmgpy/molecule/vf2.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ cdef class VF2:
if self.findAll:
mapping = {}
for vertex2 in self.graph2.vertices:
if vertex2.ignore:
continue
assert vertex2.mapping is not None
assert vertex2.mapping.mapping is vertex2
mapping[vertex2.mapping] = vertex2
Expand All @@ -178,6 +180,8 @@ cdef class VF2:
# Create list of pairs of candidates for inclusion in mapping
hasTerminals = False
for vertex2 in self.graph2.vertices:
if vertex2.ignore:
continue
if vertex2.terminal:
# graph2 has terminals, so graph1 also must have terminals
hasTerminals = True
Expand All @@ -186,6 +190,8 @@ cdef class VF2:
vertex2 = self.graph2.vertices[0]

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
Expand Down

0 comments on commit 21b83e3

Please sign in to comment.