symmetry number of phenoxy radical is 1 (should be 2) #119
Comments
enochd
added a commit
to enochd/RMG-Py
that referenced
this issue
May 16, 2014
Aromaticity is now perceived in calculating cyclic symmetry numbers. This commit copies a molecule inside `calculateCyclicSymmetryNumber`, turns it into an rdkit object, finds all the aromatic bonds and atoms. The same bonds and atoms in the corresponding rmg molecule object are then redifined so that the alogorithm is using the correct information. There is one strange thing about this fix that I dont yet understand. The unittest thinks cyclic symmetry number of dimethylbenzene is 1 (should be 2). On my local machine, the same `calculateCyclicSymmetryNumber` function gets it correct, at 2. Also, i'm inadvertantly printing a bit to stdout, and not sure why it's happening? relevant to issues ReactionMechanismGenerator#12, ReactionMechanismGenerator#24, ReactionMechanismGenerator#119, ReactionMechanismGenerator#70, ReactionMechanismGenerator#141
connie
pushed a commit
to connie/RMG-Py
that referenced
this issue
Jul 17, 2014
Aromaticity is now perceived in calculating cyclic symmetry numbers. This commit copies a molecule inside `calculateCyclicSymmetryNumber`, turns it into an rdkit object, finds all the aromatic bonds and atoms. The same bonds and atoms in the corresponding rmg molecule object are then redifined so that the alogorithm is using the correct information. There is one strange thing about this fix that I dont yet understand. The unittest thinks cyclic symmetry number of dimethylbenzene is 1 (should be 2). On my local machine, the same `calculateCyclicSymmetryNumber` function gets it correct, at 2. Also, i'm inadvertantly printing a bit to stdout, and not sure why it's happening? relevant to issues ReactionMechanismGenerator#12, ReactionMechanismGenerator#24, ReactionMechanismGenerator#119, ReactionMechanismGenerator#70, ReactionMechanismGenerator#141 fixup! added aromaticity recognition for symmetry calcs Better to avoid casting to string if we can - just check if it is None. fixup! added aromaticity recognition for symmetry calcs I think this could have been nasty! atomType is a mutable object. Suppose it was a Cds atom type. What your code would have done, is change the label on all Cds atom types to be 'Cb'. atom1 -> AtomTypeCds -> label -> 'Cds' would become atom1 -> AtomTypeCds -> label -> 'Cb' instead of atom1 -> AtomTypeCb -> label -> 'Cb' Because the AtomType objects are shared, I think it would have changed them everywhere, not just in this copy of this molecule.
enochd
added a commit
to enochd/RMG-Py
that referenced
this issue
Jul 17, 2014
Aromaticity is now perceived in calculating cyclic symmetry numbers. This commit copies a molecule inside `calculateCyclicSymmetryNumber`, turns it into an rdkit object, finds all the aromatic bonds and atoms. The same bonds and atoms in the corresponding rmg molecule object are then redifined so that the alogorithm is using the correct information. There is one strange thing about this fix that I dont yet understand. The unittest thinks cyclic symmetry number of dimethylbenzene is 1 (should be 2). On my local machine, the same `calculateCyclicSymmetryNumber` function gets it correct, at 2. Also, i'm inadvertantly printing a bit to stdout, and not sure why it's happening? relevant to issues ReactionMechanismGenerator#12, ReactionMechanismGenerator#24, ReactionMechanismGenerator#119, ReactionMechanismGenerator#70, ReactionMechanismGenerator#141
connie
pushed a commit
to connie/RMG-Py
that referenced
this issue
Jul 18, 2014
Aromaticity is now perceived in calculating cyclic symmetry numbers. This commit copies a molecule inside `calculateCyclicSymmetryNumber`, turns it into an rdkit object, finds all the aromatic bonds and atoms. The same bonds and atoms in the corresponding rmg molecule object are then redifined so that the alogorithm is using the correct information. There is one strange thing about this fix that I dont yet understand. The unittest thinks cyclic symmetry number of dimethylbenzene is 1 (should be 2). On my local machine, the same `calculateCyclicSymmetryNumber` function gets it correct, at 2. Also, i'm inadvertantly printing a bit to stdout, and not sure why it's happening? relevant to issues ReactionMechanismGenerator#12, ReactionMechanismGenerator#24, ReactionMechanismGenerator#119, ReactionMechanismGenerator#70, ReactionMechanismGenerator#141 fixup! added aromaticity recognition for symmetry calcs Better to avoid casting to string if we can - just check if it is None. fixup! added aromaticity recognition for symmetry calcs I think this could have been nasty! atomType is a mutable object. Suppose it was a Cds atom type. What your code would have done, is change the label on all Cds atom types to be 'Cb'. atom1 -> AtomTypeCds -> label -> 'Cds' would become atom1 -> AtomTypeCds -> label -> 'Cb' instead of atom1 -> AtomTypeCb -> label -> 'Cb' Because the AtomType objects are shared, I think it would have changed them everywhere, not just in this copy of this molecule.
|
i think this can be closed now |
nickvandewiele
pushed a commit
to nickvandewiele/RMG-Py
that referenced
this issue
Mar 19, 2015
Aromaticity is now perceived in calculating cyclic symmetry numbers. This commit copies a molecule inside `calculateCyclicSymmetryNumber`, turns it into an rdkit object, finds all the aromatic bonds and atoms. The same bonds and atoms in the corresponding rmg molecule object are then redifined so that the alogorithm is using the correct information. There is one strange thing about this fix that I dont yet understand. The unittest thinks cyclic symmetry number of dimethylbenzene is 1 (should be 2). On my local machine, the same `calculateCyclicSymmetryNumber` function gets it correct, at 2. Also, i'm inadvertantly printing a bit to stdout, and not sure why it's happening? relevant to issues ReactionMechanismGenerator#12, ReactionMechanismGenerator#24, ReactionMechanismGenerator#119, ReactionMechanismGenerator#70, ReactionMechanismGenerator#141 fixup! added aromaticity recognition for symmetry calcs Better to avoid casting to string if we can - just check if it is None. fixup! added aromaticity recognition for symmetry calcs I think this could have been nasty! atomType is a mutable object. Suppose it was a Cds atom type. What your code would have done, is change the label on all Cds atom types to be 'Cb'. atom1 -> AtomTypeCds -> label -> 'Cds' would become atom1 -> AtomTypeCds -> label -> 'Cb' instead of atom1 -> AtomTypeCb -> label -> 'Cb' Because the AtomType objects are shared, I think it would have changed them everywhere, not just in this copy of this molecule.
|
I can still reproduce this error on unittests. |
Merged
|
fixed with #1014 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
phenoxy:
InChI=1S/C6H5O/c7-6-4-2-1-3-5-6/h1-5H
I found 1 through rmg.mit.edu.
RMG-Java still finds 2. so this means that the algorithm for calculating symmetry numbers is changed.
The text was updated successfully, but these errors were encountered: