ReactionMechanismGenerator / RMG-Py

Python version of the amazing Reaction Mechanism Generator (RMG).
http://reactionmechanismgenerator.github.io/RMG-Py/
Other
382 stars 226 forks source link

to_single_bonds method raises AtomTypeError for molecules with "N5sc" atomtype #1863

Closed davidfarinajr closed 1 year ago

davidfarinajr commented 4 years ago

Bug Description

The to_single_bonds method raises an AtomTypeError when used for molecules with N5sc atomtype.

AtomTypeError: Unable to determine atom type for atom N, 
which has 4 single bonds, 0 double bonds to C, 0 double bonds to O, 
0 double bonds to S, 0 triple bonds, 0 benzene bonds, 0 lone pairs, and 0 charge.

How To Reproduce

from rmgpy.molecule import Molecule
mol = Molecule(smiles='[O-][NH2+]C=C')
mol.to_single_bonds()

Expected Behavior

I'm using the to_single_bonds method in AutoTST to check isomorphism from quantum chem calculations. We get the atomtype error because the to_single_bonds method does not conserve formal charges for the atoms, and N5sc is only allowed +1/+2 charges. The work around I am using now is adding 0 to allowed N5sc atomtype charges.

Installation Information

Describe your installation method and system information.

Additional Context

alongd commented 4 years ago

Will keeping the charge in to_single_bonds be correct and solve this, and can we think of cases where it won't be correct to keep them?

davidfarinajr commented 4 years ago

Will keeping the charge in to_single_bonds be correct and solve this, and can we think of cases where it won't be correct to keep them?

Yes, however I think the to_single_bonds method is primarily used to check isomorphism with a QC calc, where it is difficult to determine formal charges on the atoms. That's why I decided to add 0 to allowable atomtype charges instead of conserve atom charges with to_single_bonds method. We could keep the charges with to_single_bonds, and add an option to neglect charge with is_isomorphic method when comparing with molecule initiated with from_xyz.

github-actions[bot] commented 1 year ago

This issue is being automatically marked as stale because it has not received any interaction in the last 90 days. Please leave a comment if this is still a relevant issue, otherwise it will automatically be closed in 30 days.