choderalab / perses

Experiments with expanded ensembles to explore chemical space
http://perses.readthedocs.io
MIT License
181 stars 51 forks source link

Energy after minimizing new conformation is still being reported #256

Closed jchodera closed 8 years ago

jchodera commented 8 years ago

I think this was debug code that accidentally got left in:

Energy after minimizing the new configuration
                       HarmonicBondForce        1.245 kcal/mol
                      HarmonicAngleForce        0.902 kcal/mol
                    PeriodicTorsionForce        1.636 kcal/mol
                          NonbondedForce        7.645 kcal/mol
                         CMMotionRemover        0.000 kcal/mol

Perhaps it could at least be made optional?

pgrinaway commented 8 years ago

Ok. That sounds like a good idea. After we are finished with implementing the necessary tests, I will take a sweep through the code and fix the debug statements so that they are more easily controlled (such as through logger)

jchodera commented 8 years ago

It's not just switching it from printed to logging---in large systems, this really will slow things down.

pgrinaway commented 8 years ago

I know. But I was just grouping it with a larger set of issues involving excessive printing, and decided I would take care of it then.

jchodera commented 8 years ago

Since it could effectively prevent testing of larger systems, I imagine it's in a separate class than just "too much stuff is being printed", which may be a nuisance but not a show-stopper. Also, the solution to both is not of the same kind (shunt to logging).

As a further argument, it breaks lots of other tests right now (like run_kinase_inhibitors):

mski1776:perses.choderalab choderaj$ python perses/tests/testsystems.py
================================================================================
SAMS sampler iteration     0
--------------------------------------------------------------------------------
Expanded Ensemble sampler iteration        0
................................................................................
MCMC sampler iteration 0
Taking 50 steps of GHMC...
Using platform 'OpenCL'
potential  = -478.530 kT
kinetic    = 73.087 kT
force norm = 26.039 kT/A/dof
Accepted 49 / 50 GHMC steps (98.00%).
Finished integration in 0.460 s
Final energy is     -460.366 kT
................................................................................
Updating chemical state with ncmc-geometry-ncmc scheme...
Proposing new topology...
Warning: SelectElfDiverseConfs: elfPop.NumConfs 1 <= elfLimit 1
Proposed transformation: Cc1cccc(c1NC(=O)c2cnc(s2)Nc3cc(nc(n3)C)N4CCN(CC4)CCO)Cl => CNC(=O)c1ccccc1Sc2ccc3c(c2)[nH]nc3/C=C/c4ccccn4
Generating new system...
Performing NCMC annihilation
Geometry engine proposal...
proposal took 5.437 s
Writing proposed geometry...
Geometry engine logP_reverse calculation...
calculation took 8.426 s
Performing NCMC insertion
NCMC took 0.550 s
potential before geometry  :     -460.366 kT
                       HarmonicBondForce        9.517 kcal/mol
                      HarmonicAngleForce       16.132 kcal/mol
                    PeriodicTorsionForce       15.453 kcal/mol
                          NonbondedForce     -315.554 kcal/mol
                         CMMotionRemover        0.000 kcal/mol
potential after geometry   :     1728.869 kT
                       HarmonicBondForce      360.042 kcal/mol
                      HarmonicAngleForce      400.646 kcal/mol
                    PeriodicTorsionForce       94.507 kcal/mol
                          NonbondedForce      175.472 kcal/mol
                         CMMotionRemover        0.000 kcal/mol
  GEOMETRY ENERGY CHANGE   :    +2189.234 kT
---------------------------------------------------------
switch_logp                :    -2189.234
geometry_logp_propose      :      -59.447
geometry_logp_reverse      :      -88.578
Traceback (most recent call last):
  File "perses/tests/testsystems.py", line 2322, in <module>
    run_kinase_inhibitors()
  File "perses/tests/testsystems.py", line 2124, in run_kinase_inhibitors
    testsystem.sams_samplers[environment].run(niterations=100)
  File "/Users/choderaj/miniconda/lib/python2.7/site-packages/perses/samplers/samplers.py", line 1374, in run
    self.update()
  File "/Users/choderaj/miniconda/lib/python2.7/site-packages/perses/samplers/samplers.py", line 1357, in update
    self.update_sampler()
  File "/Users/choderaj/miniconda/lib/python2.7/site-packages/perses/samplers/samplers.py", line 1312, in update_sampler
    self.sampler.update()
  File "/Users/choderaj/miniconda/lib/python2.7/site-packages/perses/samplers/samplers.py", line 1177, in update
    self.update_state()
  File "/Users/choderaj/miniconda/lib/python2.7/site-packages/perses/samplers/samplers.py", line 988, in update_state
    ctx = openmm.Context(test_sys, integrator)
  File "/Users/choderaj/miniconda/lib/python2.7/site-packages/simtk/openmm/openmm.py", line 6250, in __init__
    this = _openmm.new_Context(*args)
Exception: A constraint cannot involve a massless particle
pgrinaway commented 8 years ago

Since it could effectively prevent testing of larger systems, I imagine it's in a separate class than just "too much stuff is being printed", which may be a nuisance but not a show-stopper.

Unfortunately, the test suite is currently very inadequate. I am currently prioritizing fixing up the test suite for GeometryEngine. After that, and before I move on to audit the tests for the Samplers, I will fix this issue.

Also, the solution to both is not of the same kind (shunt to logging).

Actually, I was considering exploring the Handler features in logging for some of this, though that may turn out to be inappropriate. In either case, I am aware.

As I said, this is an important issue, and I will address it as soon as the GeometryEngine tests are completed.

pgrinaway commented 8 years ago

Being fixed in #263.

pgrinaway commented 8 years ago

Should be resolved.

jchodera commented 8 years ago

Thanks!