choderalab / alchemy

Alchemical tools for OpenMM
9 stars 3 forks source link

Fix PME direct-space and add host-guest overlap test #46

Closed jchodera closed 8 years ago

jchodera commented 8 years ago

Fixes #45 and adds a host-guest test.

jchodera commented 8 years ago

This should be ready to review and merge.

Note that I think the overlap tests don't run on travis, so we'll want to enable jenkins testing

jchodera commented 8 years ago

Now I'm seeing a failure here---not sure which system yet:

======================================================================
ERROR: Compare annihilated electrostatics / decoupled sterics states in vacuum and a large box.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/miniconda/envs/test/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/travis/miniconda/envs/test/lib/python2.7/site-packages/alchemy-1.3.0-py2.7.egg/alchemy/tests/test_alchemy.py", line 336, in test_denihilated_states
    raise Exception("Maximum allowable difference lambda=1 energy and lambda=0 energy in vacuum and periodic box exceeded (was %.8f kcal/mol; allowed %.8f kcal/mol); test failed." % (delta / unit.kilocalories_per_mole, MAX_DELTA / unit.kilocalories_per_mole))
Exception: Maximum allowable difference lambda=1 energy and lambda=0 energy in vacuum and periodic box exceeded (was 2.74731396 kcal/mol; allowed 0.59616198 kcal/mol); test failed.
-------------------- >> begin captured logging << --------------------
jchodera commented 8 years ago

It's not printing out the logger info before the exception is thrown---how can I fix this? https://github.com/choderalab/alchemy/blob/5bea183a0a3a42a9746f9e2961f21492ee9f0920/alchemy/tests/test_alchemy.py#L326-L336

jchodera commented 8 years ago

I think the failure is likely due to the fact that PME for the ligand now is correctly tapered by erfc(alpha*r), so that the fully-interacting lambda=1 state for PME will differ from the lambda=1 state in vacuum. I imagine we can just disable this test for now and think of a better test.

jchodera commented 8 years ago

Can anyone suggest how I get the logging to print?

jchodera commented 8 years ago

I've temporarily disabled the test that was failing---which likely can no longer actually work with the PME fix. We can figure out how to fix both (1) logging and (2) that test in a separate PR.

jchodera commented 8 years ago

Ready for review by @andrrizzi (or @Lnaden) and merge!

jchodera commented 8 years ago

This should hopefully fix the PME overlap issues, or at least greatly reduce them.

andrrizzi commented 8 years ago

Awesome! Thanks for catching this. Looks great to me.

jchodera commented 8 years ago

Merged.

Should we cut a new bugfix release?

Lnaden commented 8 years ago

Yes, since this does fix a major bug in the alchemical PME treatment