MobleyLab / chemper

Repository for Chemical Perception Sampling Tools
MIT License
19 stars 10 forks source link

Remove option to remove one bond or decorator #87

Open bannanc opened 5 years ago

bannanc commented 5 years ago

While writing up the Reducer options for removing decorators it occurred to me that removing one or decorator at a time will never work on bonds.

For bonds, or'd together decorators in ChemPer are only the order of the bonds used to make the SMIRKS patterns. So removing one of those orders will never result in keeping clustering, where removing all of them could be ok.

For example, there are a fair number of SMIRKS in SMIRNOFF99Frosst which use double or aromatic bonds (=,:). If we need to keep a SMIRKS that matches both then trying to remove just the double decorator switching the bond to just aromatic doesn't make any sense. However, we can imagine a situation where the order of the bond isn't actually important. In that case we could remove all of the decorators and switch the bond to and (~).

bannanc commented 5 years ago

For reference this is here in the smirksifier.py file.

Assuming my logic on this gets verified I'll add it to PR #82.

bannanc commented 5 years ago

I think this logic is sound, and I talked through it with Vickie at some point, but I'm not going to do it in #82 because I want version 1.0.0 to have the removal options I described in our paper. I'll do this as a separate PR after that release.

bannanc commented 4 years ago

I never did this, so its up to whoever takes over