Skip to content

Added Chlorine as a new reactive element in RMG #1310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
amarkpayne merged 9 commits into from
Mar 14, 2018
Merged

Added Chlorine as a new reactive element in RMG #1310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e2bd018
Added `Cl1s` chlorine atomType
alongd Mar 6, 2018
444d908
Added Chlorine atomType to documentation
alongd Mar 6, 2018
7495ad0
Added a chlorine atomType Test
alongd Mar 6, 2018
7cd5cf5
Avoid generating a charged chlorine atomType in isomorphismTest
alongd Mar 6, 2018
6ed0c14
Added C-Cl and C-F to CBS-QB3 BAC
alongd Mar 13, 2018
4288bec
Added F, I to element.py
alongd Mar 13, 2018
87dc475
Added Cl, Si to group updateFingerprint()
alongd Mar 13, 2018
7758183
Added is F/Cl/I to Atom in molecule.py
alongd Mar 13, 2018
f317c69
Added Cl and Si to generatePairs() in reaction.py
alongd Mar 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions documentation/source/reference/molecule/atomtype.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,8 @@ Atom type Description
``S6td`` sulfur atom with no lone pairs (valance 6), one triple bond, one double bond and up to one single bond
``S6tt`` sulfur atom with no lone pairs (valance 6) and two triple bonds
``S6tdc`` charged sulfur atom with no lone pairs (valance 6), one to two triple bonds, up to two double bonds, and up to four single bonds
*Chlorine atom types*
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
``Cl`` chlorine atom with any local bond structure
``Cl1s`` chlorine atom with three lone pairs and zero to one single bonds
=============== ==============================================================================================================================================================
4 changes: 4 additions & 0 deletions documentation/source/users/rmg/database/introduction.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ table below shows all atoms types in RMG.
+----------+-------------------+------------------------------------------------------------------------------------------------------------------+
|S6tdc |Sulfur |No lone pairs, one to two triple bonds, up to two double bonds, up to four single bonds, charged -1/-1 |
+----------+-------------------+------------------------------------------------------------------------------------------------------------------+
|Cl |Chlorine |No requirements |
+----------+-------------------+------------------------------------------------------------------------------------------------------------------+
|Cl1s |Chlorine |Three lone pairs, zero to one single bonds |
+----------+-------------------+------------------------------------------------------------------------------------------------------------------+
|He |Helium |No requirements, nonreactive |
+----------+-------------------+------------------------------------------------------------------------------------------------------------------+
|Ne |Neon |No requirements, nonreactive |
Expand Down
8 changes: 4 additions & 4 deletions rmgpy/cantherm/statmech.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,10 +705,10 @@ def applyEnergyCorrections(E0, modelChemistry, atoms, bonds):
'N-H': 0.06, 'N-N': -0.23, 'N=N': -0.37, 'N#N': -0.64,}
elif modelChemistry == 'CBS-QB3':
bondEnergies = {
'C-C': -0.495, 'C-H': -0.045, 'C=C': -0.825, 'C-O': 0.378, 'C=O': 0.743, 'O-H': -0.423, #Table2: Paraskevas, PD (2013). Chemistry-A European J., DOI: 10.1002/chem.201301381
'C#C': -0.64, 'C#N': -0.89, 'C-S': 0.43, 'O=S': -0.78, 'S-H': 0.0, 'C-N': -0.13, # Table IX: Petersson GA (1998) J. of Chemical Physics, DOI: 10.1063/1.477794
'N-H': -0.42, 'N=O': 1.11, 'N-N': -1.87, 'N=N': -1.58, 'N-O': 0.35, #Table 2: Ashcraft R (2007) J. Phys. Chem. B; DOI: 10.1021/jp073539t
'N#N': -2.0, 'O=O': -0.2, 'H-H': 1.1, # Unknown source
'C-C': -0.495,'C-H': -0.045,'C=C': -0.825,'C-O': 0.378,'C=O': 0.743,'O-H': -0.423, #Table2: Paraskevas, PD (2013). Chemistry-A European J., DOI: 10.1002/chem.201301381
Copy link
Member

@amarkpayne amarkpayne Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like your editor changed some of the spacing here

Copy link
Member Author

@alongd alongd Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was actually me, trying to make the lines a bit shorter (now with the 'C-Cl', 'C-F' additions, the second line was too long). It still functions good and it's readable, let me know if you think otherwise.

Copy link
Member

@amarkpayne amarkpayne Mar 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, this is fine by me then, just making sure

'C#C': -0.64, 'C#N': -0.89, 'C-S': 0.43, 'O=S': -0.78,'S-H': 0.0, 'C-N': -0.13, 'C-Cl': 1.29, 'C-F': 0.55, # Table IX: Petersson GA (1998) J. of Chemical Physics, DOI: 10.1063/1.477794
'N-H': -0.42, 'N=O': 1.11, 'N-N': -1.87, 'N=N': -1.58,'N-O': 0.35, #Table 2: Ashcraft R (2007) J. Phys. Chem. B; DOI: 10.1021/jp073539t
'N#N': -2.0, 'O=O': -0.2, 'H-H': 1.1, # Unknown source
}
elif modelChemistry in ['B3LYP/cbsb7', 'B3LYP/6-311G(2d,d,p)', 'DFT_G03_b3lyp','B3LYP/6-311+G(3df,2p)','b3lyp/6-31G**']:
bondEnergies = { 'C-H': 0.25, 'C-C': -1.89, 'C=C': -0.40, 'C#C': -1.50,
Expand Down
38 changes: 19 additions & 19 deletions rmgpy/molecule/atomtype.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,25 +235,24 @@ def getFeatures(self):
'H',
'R!H',
'Val4','Val5','Val6','Val7',
'He',
'He','Ne','Ar',
'C','Ca','Cs','Csc','Cd','CO','CS','Cdd','Cdc','Ct','Cb','Cbf','C2s','C2sc','C2d','C2dc','C2tc',
'N','N0sc','N1s','N1sc','N1dc','N3s','N3sc','N3d','N3t','N3b','N5sc','N5dc','N5ddc','N5dddc','N5t','N5tc','N5b','N5bd',
'O','Oa','O0sc','O2s','O2sc','O2d','O4sc','O4dc','O4tc','O4b',
'Ne',

'Si','Sis','Sid','Sidd','Sit','SiO','Sib','Sibf',
'S','Sa','S0sc','S2s','S2sc','S2d','S2dc','S2tc','S4s','S4sc','S4d','S4dd','S4dc','S4b','S4t','S4tdc','S6s','S6sc','S6d','S6dd','S6ddd','S6dc','S6t','S6td','S6tt','S6tdc',
'Cl','Ar']
)
'Cl','Cl1s'])

atomTypes['R!H'] = AtomType(label='R!H', generic=['R'], specific=[
'He',
'Val4','Val5','Val6','Val7',
'He','Ne','Ar',
'C','Ca','Cs','Csc','Cd','CO','CS','Cdd','Cdc','Ct','Cb','Cbf','C2s','C2sc','C2d','C2dc','C2tc',
'N','N0sc','N1s','N1sc','N1dc','N3s','N3sc','N3d','N3t','N3b','N5sc','N5dc','N5ddc','N5dddc','N5t','N5tc','N5b','N5bd',
'O','Oa','O0sc','O2s','O2sc','O2d','O4sc','O4dc','O4tc','O4b',
'Ne',
'Si','Sis','Sid','Sidd','Sit','SiO','Sib','Sibf',
'S','Sa','S0sc','S2s','S2sc','S2d','S2dc','S2tc','S4s','S4sc','S4d','S4dd','S4dc','S4b','S4t','S4tdc','S6s','S6sc','S6d','S6dd','S6ddd','S6dc','S6t','S6td','S6tt','S6tdc',
'Cl','Ar'])
'Cl','Cl1s'])

atomTypes['Val4'] = AtomType(label='Val4', generic=['R','R!H'], specific=[
'C','Ca','Cs','Csc','Cd','CO','CS','Cdd','Cdc','Ct','Cb','Cbf','C2s','C2sc','C2d','C2dc','C2tc',
Expand All @@ -267,11 +266,13 @@ def getFeatures(self):
'S','Sa','S0sc','S2s','S2sc','S2d','S2dc','S2tc','S4s','S4sc','S4d','S4dd','S4dc','S4b','S4t','S4tdc','S6s','S6sc','S6d','S6dd','S6ddd','S6dc','S6t','S6td','S6tt','S6tdc'])

atomTypes['Val7'] = AtomType(label='Val7', generic=['R','R!H'], specific=[
'Cl'])
'Cl','Cl1s'])

atomTypes['H' ] = AtomType('H', generic=['R'], specific=[])

atomTypes['He' ] = AtomType('He', generic=['R','R!H'], specific=[])
atomTypes['He' ] = AtomType('He', generic=['R','R!H'], specific=[])
atomTypes['Ne' ] = AtomType('Ne', generic=['R','R!H'], specific=[])
atomTypes['Ar' ] = AtomType('Ar', generic=['R','R!H'], specific=[])

atomTypes['C' ] = AtomType('C', generic=['R','R!H','Val4'], specific=['Ca','Cs','Csc','Cd','CO','CS','Cdd','Cdc','Ct','Cb','Cbf','C2s','C2sc','C2d','C2dc','C2tc'],
single=[], allDouble=[], rDouble=[], oDouble=[], sDouble=[], triple=[], benzene=[], lonePairs=[], charge=[])
Expand Down Expand Up @@ -408,7 +409,6 @@ def getFeatures(self):
single=[0], allDouble=[0], rDouble=[0], oDouble=[0], sDouble=[0], triple=[0], benzene=[2], lonePairs=[1], charge=[0])
# examples for S4b: Furane, Benzofurane, Benzo[c]thiophene, Oxazole...

atomTypes['Ne' ] = AtomType('Ne', generic=['R','R!H'], specific=[])
atomTypes['Si' ] = AtomType('Si', generic=['R','R!H','Val4'], specific=['Sis','Sid','Sidd','Sit','SiO','Sib','Sibf'],
single=[], allDouble=[], rDouble=[], oDouble=[], sDouble=[], triple=[], benzene=[], lonePairs=[], charge=[])
atomTypes['Sis' ] = AtomType('Sis', generic=['R','R!H','Si','Val4'], specific=[],
Expand Down Expand Up @@ -504,9 +504,11 @@ def getFeatures(self):
single=[0,1,2,3,4], allDouble=[0,1,2], rDouble=[], oDouble=[], sDouble=[], triple=[1,2], benzene=[0], lonePairs=[0], charge=[-1,+1])
# *Composite atomType; examples for S6tdc: [SH2+]#[C-], [N-]=[S+]#N

atomTypes['Cl' ] = AtomType('Cl', generic=['R','R!H','Val7'], specific=[])
atomTypes['Cl' ] = AtomType('Cl', generic=['R','R!H','Val7'], specific=['Cl1s'])
atomTypes['Cl1s'] = AtomType('Cl1s', generic=['R','R!H','Cl','Val7'], specific=[],
single=[0,1], allDouble=[0], rDouble=[], oDouble=[], sDouble=[], triple=[0], benzene=[0], lonePairs=[3], charge=[0])
# examples for Cl1s: HCl, [Cl]

atomTypes['Ar' ] = AtomType('Ar', generic=['R','R!H'], specific=[])

atomTypes['R' ].setActions(incrementBond=['R'], decrementBond=['R'], formBond=['R'], breakBond=['R'], incrementRadical=['R'], decrementRadical=['R'], incrementLonePair=['R'], decrementLonePair=['R'])
atomTypes['R!H' ].setActions(incrementBond=['R!H'], decrementBond=['R!H'], formBond=['R!H'], breakBond=['R!H'], incrementRadical=['R!H'], decrementRadical=['R!H'], incrementLonePair=['R!H'], decrementLonePair=['R!H'])
Expand All @@ -515,10 +517,11 @@ def getFeatures(self):
atomTypes['Val6'].setActions(incrementBond=['Val6'], decrementBond=['Val6'], formBond=['Val6'], breakBond=['Val6'], incrementRadical=['Val6'], decrementRadical=['Val6'], incrementLonePair=['Val6'],decrementLonePair=['Val6'])
atomTypes['Val7'].setActions(incrementBond=['Val7'], decrementBond=['Val7'], formBond=['Val7'], breakBond=['Val7'], incrementRadical=['Val7'], decrementRadical=['Val7'], incrementLonePair=['Val7'],decrementLonePair=['Val7'])


atomTypes['H' ].setActions(incrementBond=[], decrementBond=[], formBond=['H'], breakBond=['H'], incrementRadical=['H'], decrementRadical=['H'], incrementLonePair=[], decrementLonePair=[])

atomTypes['He' ].setActions(incrementBond=[], decrementBond=[], formBond=[], breakBond=[], incrementRadical=['He'], decrementRadical=['He'], incrementLonePair=[], decrementLonePair=[])
atomTypes['Ne' ].setActions(incrementBond=[], decrementBond=[], formBond=[], breakBond=[], incrementRadical=['Ne'], decrementRadical=['Ne'], incrementLonePair=[], decrementLonePair=[])
atomTypes['Ar' ].setActions(incrementBond=[], decrementBond=[], formBond=[], breakBond=[], incrementRadical=[], decrementRadical=[], incrementLonePair=[], decrementLonePair=[])

atomTypes['C' ].setActions(incrementBond=['C'], decrementBond=['C'], formBond=['C'], breakBond=['C'], incrementRadical=['C'], decrementRadical=['C'], incrementLonePair=['C'], decrementLonePair=['C'])
atomTypes['Ca' ].setActions(incrementBond=[], decrementBond=[], formBond=[], breakBond=[], incrementRadical=[], decrementRadical=[], incrementLonePair=[], decrementLonePair=['C2s'])
Expand Down Expand Up @@ -568,8 +571,6 @@ def getFeatures(self):
atomTypes['O4tc'].setActions(incrementBond=[], decrementBond=['O4dc'], formBond=[], breakBond=[], incrementRadical=[], decrementRadical=[], incrementLonePair=['O2d'], decrementLonePair=[])
atomTypes['O4b' ].setActions(incrementBond=[], decrementBond=[], formBond=[], breakBond=[], incrementRadical=[], decrementRadical=[], incrementLonePair=[], decrementLonePair=[])

atomTypes['Ne' ].setActions(incrementBond=[], decrementBond=[], formBond=[], breakBond=[], incrementRadical=['Ne'], decrementRadical=['Ne'], incrementLonePair=[], decrementLonePair=[])

atomTypes['Si' ].setActions(incrementBond=['Si'], decrementBond=['Si'], formBond=['Si'], breakBond=['Si'], incrementRadical=['Si'], decrementRadical=['Si'], incrementLonePair=[], decrementLonePair=[])
atomTypes['Sis' ].setActions(incrementBond=['Sid','SiO'], decrementBond=[], formBond=['Sis'], breakBond=['Sis'], incrementRadical=['Sis'], decrementRadical=['Sis'], incrementLonePair=[], decrementLonePair=[])
atomTypes['Sid' ].setActions(incrementBond=['Sidd','Sit'], decrementBond=['Sis'], formBond=['Sid'], breakBond=['Sid'], incrementRadical=['Sid'], decrementRadical=['Sid'], incrementLonePair=[], decrementLonePair=[])
Expand Down Expand Up @@ -606,15 +607,14 @@ def getFeatures(self):
atomTypes['S6tt'].setActions(incrementBond=[], decrementBond=['S6td','S6tdc'], formBond=[], breakBond=[], incrementRadical=[], decrementRadical=[], incrementLonePair=[], decrementLonePair=[])
atomTypes['S6tdc'].setActions(incrementBond=['S6td','S6tdc','S6tt'],decrementBond=['S6dc','S6tdc'],formBond=['S6tdc'],breakBond=['S6tdc'], incrementRadical=['S6tdc'],decrementRadical=['S6tdc'],incrementLonePair=['S4t','S4tdc'],decrementLonePair=[])

atomTypes['Cl' ].setActions(incrementBond=[], decrementBond=[], formBond=['Cl'], breakBond=['Cl'], incrementRadical=['Cl'], decrementRadical=['Cl'], incrementLonePair=[], decrementLonePair=[])
atomTypes['Cl1s'].setActions(incrementBond=[], decrementBond=[], formBond=['Cl1s'], breakBond=['Cl1s'], incrementRadical=['Cl1s'], decrementRadical=['Cl1s'], incrementLonePair=[], decrementLonePair=[])

atomTypes['Cl' ].setActions(incrementBond=[], decrementBond=['Cl'], formBond=['Cl'], breakBond=['Cl'], incrementRadical=['Cl'], decrementRadical=['Cl'], incrementLonePair=[], decrementLonePair=[])

atomTypes['Ar' ].setActions(incrementBond=[], decrementBond=[], formBond=[], breakBond=[], incrementRadical=[], decrementRadical=[], 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',]
nonSpecifics=['H', 'He', 'Ne', 'Cl', 'Ar',]
nonSpecifics=['H', 'He', 'Ne', 'Ar',]

for atomType in atomTypes.values():
for items in [atomType.generic, atomType.specific,
Expand Down
11 changes: 10 additions & 1 deletion rmgpy/molecule/atomtypeTest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,9 @@ def setUp(self):
# 10 H u0 p0 {4,S}
# 11 H u0 p0 {5,S}''')

self.mol73 = Molecule().fromAdjacencyList('''1 H u0 p0 c0 {2,S}
2 Cl u0 p3 c0 {1,S}''')

def atomType(self, mol, atomID):
atom = mol.atoms[atomID]
type = getAtomType(atom, mol.getBonds(atom))
Expand Down Expand Up @@ -604,7 +607,13 @@ def testSulfurTypes(self):
self.assertEqual(self.atomType(self.mol36, 0), 'S6td')
self.assertEqual(self.atomType(self.mol37, 1), 'S6tt')
self.assertEqual(self.atomType(self.mol70, 0), 'S6tdc')


def testChlorineTypes(self):
"""
Test that getAtomType() returns appropriate chlorine atom types.
"""
self.assertEqual(self.atomType(self.mol73, 1), 'Cl1s')

def testOtherTypes(self):
"""
Test that getAtomType() returns appropriate types for other misc inerts.
Expand Down
6 changes: 3 additions & 3 deletions rmgpy/molecule/element.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ class PeriodicSystem(object):
`lone_pairs`: the number of lone pairs an element has
"""

valences = {'H': 1, 'He': 0, 'C': 4, 'N': 3, 'O': 2, 'Ne': 0, 'Si': 4, 'S': 2, 'Cl': 1, 'Ar': 0}
valence_electrons = {'H': 1, 'He': 2, 'C': 4, 'N': 5, 'O': 6, 'Ne': 8, 'Si': 4, 'S': 6, 'Cl': 7, 'Ar': 8}
lone_pairs = {'H': 0, 'He': 1, 'C': 0, 'N': 1, 'O': 2, 'Ne': 4, 'Si': 0, 'S': 2, 'Cl': 3, 'Ar': 4}
valences = {'H': 1, 'He': 0, 'C': 4, 'N': 3, 'O': 2, 'F': 1, 'Ne': 0, 'Si': 4, 'S': 2, 'Cl': 1, 'Ar': 0, 'I': 1}
valence_electrons = {'H': 1, 'He': 2, 'C': 4, 'N': 5, 'O': 6, 'F': 7, 'Ne': 8, 'Si': 4, 'S': 6, 'Cl': 7, 'Ar': 8, 'I': 7}
lone_pairs = {'H': 0, 'He': 1, 'C': 0, 'N': 1, 'O': 2, 'F': 3, 'Ne': 4, 'Si': 0, 'S': 2, 'Cl': 3, 'Ar': 4, 'I': 3}

################################################################################

Expand Down
2 changes: 2 additions & 0 deletions rmgpy/molecule/group.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ cdef class Group(Graph):
cdef public short nitrogenCount
cdef public short oxygenCount
cdef public short sulfurCount
cdef public short chlorineCount
cdef public short siliconCount
cdef public short radicalCount

cpdef addAtom(self, GroupAtom atom)
Expand Down
40 changes: 26 additions & 14 deletions rmgpy/molecule/group.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1117,18 +1117,23 @@ def updateFingerprint(self):
isomorphism checks.
"""
cython.declare(atom=GroupAtom, atomType=AtomType)
cython.declare(carbon=AtomType, nitrogen=AtomType, oxygen=AtomType, sulfur=AtomType)
cython.declare(isCarbon=cython.bint, isNitrogen=cython.bint, isOxygen=cython.bint, isSulfur=cython.bint, radical=cython.int)

cython.declare(carbon=AtomType, nitrogen=AtomType, oxygen=AtomType, sulfur=AtomType, chlorine=AtomType, silicon=AtomType)
cython.declare(isCarbon=cython.bint, isNitrogen=cython.bint, isOxygen=cython.bint, isSulfur=cython.bint,
isChlorine=cython.bint, isSilicon=cython.bint, radical=cython.int)

carbon = atomTypes['C']
nitrogen = atomTypes['N']
oxygen = atomTypes['O']
sulfur = atomTypes['S']
chlorine = atomTypes['Cl']
silicon = atomTypes['Si']

self.carbonCount = 0
self.nitrogenCount = 0
self.oxygenCount = 0
self.sulfurCount = 0
self.chlorineCount = 0
self.siliconCount = 0
self.radicalCount = 0
for atom in self.vertices:
if len(atom.atomType) == 1:
Copy link
Member

@amarkpayne amarkpayne Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is outside the scope of your commit, but what should happen if len(atom.atomType) != 1?

Copy link
Member Author

@alongd alongd Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that since it is a Group and not a Molecule, an "atom" can be defined as 1 [C,N,O] ux px cx {2,S}, in which case we can't fingerprint it and we correctly skip this condition. I think this also means we're missing out on cases like 1 [Cs,Cd] ux px cx {2,S} where we could fingerprint, but don't (for that atom in the Group). However, this should be working smoothly for Molecules.

Copy link
Member

@amarkpayne amarkpayne Mar 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I wonder if there is a way to catch cases like 1 [Cs,Cd] ux px cx {2,S} and if that would be useful. If so this is something for another PR though.

Expand All @@ -1137,17 +1142,24 @@ def updateFingerprint(self):
isNitrogen = atomType.equivalent(nitrogen)
isOxygen = atomType.equivalent(oxygen)
isSulfur = atomType.equivalent(sulfur)
if isCarbon and not isNitrogen and not isOxygen and not isSulfur:
self.carbonCount += 1
elif isNitrogen and not isCarbon and not isOxygen and not isSulfur:
self.nitrogenCount += 1
elif isOxygen and not isCarbon and not isNitrogen and not isSulfur:
self.oxygenCount += 1
elif isSulfur and not isCarbon and not isNitrogen and not isOxygen:
self.sulfurCount += 1
if len(atom.radicalElectrons) == 1:
radical = atom.radicalElectrons[0]
self.radicalCount += radical
isChlorine = atomType.equivalent(chlorine)
isSilicon = atomType.equivalent(silicon)
sum_is_atom = isCarbon + isNitrogen + isOxygen + isSulfur + isChlorine + isSilicon
if sum_is_atom == 1:
Copy link
Member

@amarkpayne amarkpayne Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if sum_is_atom != 1? I can't imagine this happening, but maybe I am missing a subtlety here. I would think that this would either always evaluate to True (and thus it is not needed) or we need some sort of error handling if it does occur.

Copy link
Member Author

@alongd alongd Mar 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is related to the previous comment: an "atom" in a Group could be defined as 1 [C,N,O] ux px cx {2,S}. In this case, atomType = [C,N,O], and .equivalent will return True for more than one isAtom. In this case, sum_is_atom will be 3.
I agree that it seems unnecessary (now that I rethink it) since it's already under the if len(atom.atomType) == 1: condition discussed above. We could either keep it to keep the same functionality as before in case I'm also missing a subtlety, or remove. I'll check.

if isCarbon:
self.carbonCount += 1
elif isNitrogen:
self.nitrogenCount += 1
elif isOxygen:
self.oxygenCount += 1
elif isSulfur:
self.sulfurCount += 1
elif isChlorine:
self.chlorineCount += 1
elif isSilicon:
self.siliconCount += 1
if len(atom.radicalElectrons) >= 1:
self.radicalCount += atom.radicalElectrons[0]

def isIsomorphic(self, other, initialMap=None):
"""
Expand Down
Loading