DHFR no swapping - Githubissues

NelsonRamallo commented 5 years ago

I{m having problems making the salt swapping work consistently. I have tried running the test systems provided (in testsystems), in particular DHFR at the same conditions shown in the published article. When running the script with a non-bonded cutoff distance of 10A and the following conditions: (c=100mM) -c 0.1 -o out --ipdb minimized_dhfr.pdb --prmtop prmtop --water_name HOH -i 7500 -s 2000 -e 2000 --npert 1000 --nprop 10 --timestep 2.0 --save_freq 4 --platform CUDA I get no swaping 10a If I change the cutoff distance to 10.1A, swapping is recovered but the distribution of concentrations and the correlation time are off when compared to the results published: 101a I'd like to mention that the system also fails to get swapping when using cutoff distances of: 12A,14A. I'll attach all the files used.

dhfr.zip

jchodera commented 5 years ago

Thanks for posting this! I just realized nobody in the group was watching the repo, so thanks for the email nudge as well.

Can you give me some more information about your environment to help us debug?

Which CUDA version are you using, and what GPU?
What version of Python?
Can you dump your conda environment with conda env export > environment.yml, and then share the environment file?

jchodera commented 5 years ago

I get an Exception with the latest openmmtools, though I think this gives me a hint as to what is wrong:

[chodera@lilac:saltswap-debug]$ python sample_amber_system.py -c 0.1 -o out --ipdb minimized_dhfr.pdb --prmtop prmtop --water_name HOH -i 7500 -s 2000 -e 2000 --npert 1000 --nprop 10 --timestep 2.0 --save_freq 4 --platform CUDA

/home/chodera/miniconda/lib/python3.6/site-packages/openmmtools-0.18.0-py3.6-linux-x86_64.egg/openmmtools/multistate/__init__.py:75: UserWarning: Warning: openmmtools.multistate API is experimental
  warnings.warn('Warning: openmmtools.multistate API is experimental')
Illegal global variable name: first_step
Traceback (most recent call last):
  File "/home/chodera/miniconda/lib/python3.6/site-packages/saltswap-0.5.2-py3.6.egg/saltswap/swapper.py", line 652, in attempt_identity_swap
    work_measurement=self.work_measurement)
  File "/home/chodera/miniconda/lib/python3.6/site-packages/saltswap-0.5.2-py3.6.egg/saltswap/swapper.py", line 524, in _ncmc
    self.ncmc_integrator.setGlobalVariableByName("first_step", 0)
  File "/home/chodera/miniconda/lib/python3.6/site-packages/simtk/openmm/openmm.py", line 18629, in setGlobalVariableByName
    return _openmm.CustomIntegrator_setGlobalVariableByName(self, name, value)
Exception: Illegal global variable name: first_step

My guess is that I need to fix and merge this PR and then change the lines that call

self.ncmc_integrator.setGlobalVariableByName("first_step", 0)

to instead call

self.ncmc_integrator.reset()

I'll put this on my TODO list, but it might take a few days for me to get to it.

TienMPhan commented 5 years ago

Here are some details about our environment:

CUDA 9.2, we ran most of the jobs on GTX 1080Ti, and some on GTX 980Ti,
Python 3.6.7, OpenMM 7.3.0, openmmtools 0.16, longer list in the attached file.

environment.zip

We also tried the saltswap package with a homodimer 2mg4 system, and only had salt swapping with the non-bonded cutoff = 11A, 13A, 15A.

Below is the result for 13A

n_1_13A

The salt concentration fluctuations are very low compared with the macroscopic concentrations.

We will run it again when you have a chance to fix it.

jchodera commented 5 years ago

Hm, we fixed a bug relevant to NCMC in OpenMM 7.3.1. Could you give this a try with OpenMM 7.3.1 as well?

TienMPhan commented 5 years ago

I still get an Exception with OpenMM 7.3.1 as you mentioned above:

Illegal global variable name: first_step
Traceback (most recent call last):
  File "/homes/minhtien/.conda/envs/openmm/lib/python3.7/site-packages/saltswap-0.5.2-py3.7.egg/saltswap/swapper.py", line 652, in attempt_identity_swap
    work_measurement=self.work_measurement)
  File "/homes/minhtien/.conda/envs/openmm/lib/python3.7/site-packages/saltswap-0.5.2-py3.7.egg/saltswap/swapper.py", line 524, in _ncmc
    self.ncmc_integrator.setGlobalVariableByName("first_step", 0)
  File "/homes/minhtien/.conda/envs/openmm/lib/python3.7/site-packages/simtk/openmm/openmm.py", line 18629, in setGlobalVariableByName
    return _openmm.CustomIntegrator_setGlobalVariableByName(self, name, value)
Exception: Illegal global variable name: first_step

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "small_box.py", line 185, in <module>
    salinator.update(nattempts=1)
  File "/homes/minhtien/.conda/envs/openmm/lib/python3.7/site-packages/saltswap-0.5.2-py3.7.egg/saltswap/wrappers.py", line 405, in update
    self.swapper.update(self.context, nattempts=nattempts, cost=chemical_potential, saltmax=saltmax)
  File "/homes/minhtien/.conda/envs/openmm/lib/python3.7/site-packages/saltswap-0.5.2-py3.7.egg/saltswap/swapper.py", line 911, in update
    self.attempt_identity_swap(context, penalty=cost, saltmax=saltmax)
  File "/homes/minhtien/.conda/envs/openmm/lib/python3.7/site-packages/saltswap-0.5.2-py3.7.egg/saltswap/swapper.py", line 657, in attempt_identity_swap
    if detail[0] == 'Particle coordinate is nan':
TypeError: 'Exception' object is not subscriptable

the evironment file: environment.yml.zip

jchodera commented 5 years ago

I'm confused here, since it looks like this issue was already fixed in https://github.com/choderalab/openmmtools/commit/17576dba6f3365cb192ad0ff6fe884bda694eae4, which would have been released in 0.17 and later releases of openmmtools. Could you conda list openmmtools to see which version you have, and try a conda update openmmtools to try the latest version?

TienMPhan commented 5 years ago

It's strange, since the Exception went with OpenMM 7.3.1 and openmmtools 0.17.

(openmm) [minhtien@eos ~]$ conda list openmmtools
# packages in environment at /homes/minhtien/.conda/envs/openmm:
#
# Name                    Version                   Build  Channel
openmmtools               0.17.0                   py37_0    omnia

I get the same error on local machines as well as on a cluster and everything was updated to the latest version.

TienMPhan commented 5 years ago

I suspect the error first_step also comes from saltswap/swapper.py line 524

if work_measurement == 'internal':
   self.ncmc_integrator.setGlobalVariableByName("first_step", 0)
   # Propagation
   self.ncmc_integrator.step(nprop)

Simply changing first_step to step in saltswap/swapper.py line 524 may resolve the issue?

jchodera commented 5 years ago

Good catch! This should now be calling self.ncmc_integrator.reset() once I get the new openmmtools cut.

I'll test that and cut a new bugfix release if we can get this to work.

jchodera commented 5 years ago

I've nearly finished https://github.com/choderalab/openmmtools/pull/274

Once that is merged, I can cut a new bugfix release of openmmtools and update saltswap to use integrator.reset(). That should get this working again!

choderalab / saltswap

DHFR no swapping #29