search

openforcefield / smarty

Chemical perception tree automated exploration tool.
http://openforcefield.org
MIT License
19 stars 8 forks source link

Update smirnoff99Frosst to remove generics in DrugBank #232

Closed bannanc closed 7 years ago

bannanc commented 7 years ago

This pull request will include parameters needed to match most of the chemistry in the larger set DrugBank. It still is no metals and molecules with <= 200 heavy atoms.

In this message I will list all of the changes I've made with links to issues where they have been discussed.

Parm99 or Parm@Frosst

Bonds

Angles

Torsions

Estimates from GAFF2

Bonds

Angles

Torsions

davidlmobley commented 7 years ago

@bannanc - I'm going to suggest edits to your summary above since this is our record for posterity. Here are a couple of things which are unclear to me:

Our original plan was to remove the +0, however, there were bonds specified above for the case of #16+1 to carbon so to avoid overwriting those, the carbon bond was changed to [#16X4,#16X3!+1]

What's "specified above" mean? I'm guessing you mean "above this in the force field file"? Also, the reference of "the carbon bond" could be more clear -- THIS carbon bond? Or the one above? (Again, presumably you mean the former, but it's easier if it's just clear when you write it the first time so one doesn't have to stop and think about it.)

bannanc commented 7 years ago

@davidlmobley Assuming you approve the newest round of changes. This now types all the filtered drugbank molecules, that have tripos mol2 files.

Note - there are a few corner cases we don't quite catch in the full DrugBank set.

My next plan is to close all the issues this pull request addresses and then open a single (LONG) issue with chemistry that we think needs to be explored further. That will include anything we got from GAFF2, things we don't cover now (i.e. boron/silicon), and anything where we made things fairly generic to cover our bases. This will serve as a source of molecules/functional groups for the undergrad in our group during the summer.

bannanc commented 7 years ago

For reference:

Charged Tripos mol2 files of filtered drugBank

GAFF2 typed molecules for filtered DrugBank

davidlmobley commented 7 years ago

@bannanc :

My next plan is to close all the issues this pull request addresses and then open a single (LONG) issue with chemistry that we think needs to be explored further. That will include anything we got from GAFF2, things we don't cover now (i.e. boron/silicon), and anything where we made things fairly generic to cover our bases. This will serve as a source of molecules/functional groups for the undergrad in our group during the summer.

This plan sounds excellent.

Reviewing FF changes in a cursory way (since I already reviewed all the individual issues in detail) now.

davidlmobley commented 7 years ago

And, once merged, you will also migrate these changes to github.com/open-forcefield-group/openforcefield and github.com/open-forcefield-group/smirnoff99frosst, @bannanc ? (We're working towards removing smirnoff from smarty).

bannanc commented 7 years ago

@davidlmobley if you're ok with what I did with the aromatic nitrogen torsion then I think this is good to know with the knowledge that we'll probably have another one in the not too distant future.

davidlmobley commented 7 years ago

@bannanc - tests are failing because the tests load smirff99Frosst.ffxml but you renamed to smirnoff99Frosst.ffxml. Can you fix?

Failure: ValueError (Sorry! /home/travis/miniconda/envs/test/lib/python2.7/site-packages/smarty/data/forcefield/smirff99Frosst.ffxml does not exist. If you just added it, you'll have to re-install) ... ERROR

davidlmobley commented 7 years ago

(It's OK with me if you go ahead and merge once tests pass, @bannanc !)

davidlmobley commented 7 years ago

@bannanc - looks like py27 and py35 passed on your last commit, but py34 is failing. Checking.

bannanc commented 7 years ago

The most recent run failed on python3 and not python2, but the failure said it didn't get 100% on the AlkEthOH answers... I don't think we changed anything in the AlkEthOH parameters on this set. I'll see if the same problem repeats on this test

davidlmobley commented 7 years ago

Well, that's really weird... Py34 test failed with something you haven't touched in here:

FAIL: Test score is 100% if correct VdW, Bond, Angles, or Torsions
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/miniconda/envs/test/lib/python3.4/site-packages/smarty/tests/test_smirky_sampler.py", line 53, in test_correct_fragments
    self.assertAlmostEqual(fracfound, 1.0, msg = "Not finding 100%% of AlkEthOH when starting from correct %s SMIRKS." % typetag)
AssertionError: 0.9962640099626401 != 1.0 within 7 places : Not finding 100% of AlkEthOH when starting from correct VdW SMIRKS.

I think the SMIRKY test may have accepted a very rare move which resulted in losing some coverage of the AlkEthOH set, perhaps, just in this one case? Looks like it should be good to merge when you are ready, @bannanc .

bannanc commented 7 years ago

~I'm going to make the number of steps on the 100% test smaller to avoid that happening in the future.~

Nevermind, it already has 2 steps and temperature = 0.0 it shouldn't ever accept moves that decrease the score... I don't know what caused the failed test assuming this one passes I'll merge it.