MobleyLab / chemper

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

Fix symmetry for torsions #60

Closed bannanc closed 5 years ago

bannanc commented 5 years ago

In this pull request I'm primarily addressing issue #59. To do this, I've expanded the handling of paired atoms and bonds from molecules with the atoms and bonds stored in the graph in cluster_graph.
Specifically, I made it so you could send an arbitrary number of atoms and bonds for pairing in find_pairs. This lets you use the typical atom and bond pair used for normal layers, but then use only 1 atom in bond symmetry around a central bond or an atom-bond-atom set for proper torsions with symmetry around the central bond.

I started by writing a test for comparing chemper SMIRKS to smirnoff99Frosst with the AlkEthOH molecule set. This test is passing locally. I will upload momentarily.

However, it appears that chemper is still failing with MiniDrugBank (including fairly minimal subsets), so I will continue to investigate problems.

Per earlier documentation, I am trying to put one issue per PR and then merging when tests pass with the "Review" label left until @jmaat has time to review. So I will merge this PR when tests pass here and then make an new issue and PR for any future discovered bugs.

codecov[bot] commented 5 years ago

Codecov Report

Merging #60 into master will increase coverage by 2.58%. The diff coverage is 94.44%.

bannanc commented 5 years ago

On the off chance Jessica catches up with me, I'll leave this open until I find what's causing problems with MiniDrugBank.

bannanc commented 5 years ago

@jmaat I'm going to merge this one also. It will be easier for trying out Chaya's torsion data if this is in the master branch. I also want to try a few things that might need a few separate branches and once again it will be easier if these changes are in master already.