Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Continuing a Path Sampling Simulation with GROMACS #1101

Open
jjfsteuer opened this issue Jan 27, 2022 · 7 comments
Open

Continuing a Path Sampling Simulation with GROMACS #1101

jjfsteuer opened this issue Jan 27, 2022 · 7 comments

Comments

@jjfsteuer
Copy link

First of all a big thanks to the developers for the great software, documentation and continuous development.

I am currently trying to continue a TPS simulation using OPS with the GROMACS Engine.

My initial run consists of 100 steps, and to append to that simulation storage I use the following code:

storage = paths.Storage(filename, "a")
tps_calc = storage.pathsimulators[0]
tps_calc.restart_at_step(storage.steps[-1]) #
tps_calc.run(200)
storage.close()

While the OPS package appears to do what I expect

Working on Monte Carlo cycle number 101
Starting simulation...
Working on first step

the executed GROMACS command does not adopt the corresponding index

gmx -nobackup mdrun -s topol.tpr -o ././prod_trr/0000001.trr -e ././prod_edr/0000001.edr -g ././prod_log/0000001.log

and the trajectory files from the initial run are overwritten.

I was able to reproduce the same problem for the alanine dipeptide example for GROMACS (trying to continue from the simulation in AD_tps_2a_run_flex.ipynb).
Again, the appending run appears to continue with the TPS where it is supposed to, but the GROMACS filenames are restarted at 0000001.

Any advice or suggestion what might be going wrong is greatly appreciated!
Best,
Jakob

@sroet
Copy link
Member

sroet commented Jan 27, 2022

Thanks for raising this issue!

First,

the quick and dirty solution:

change your example code to

storage = paths.Storage(filename, "a")
tps_calc = storage.pathsimulators[0]
tps_calc.restart_at_step(storage.steps[-1]) #

num_steps = len(storage.steps)

# This does not have to be a loop if you know which engine object you are using for your simulation
for eng in storage.engines:
    eng.filename_setter.count = num_steps

tps_calc.run(200)
storage.close()

why this happens

OPS stores things like the engine only once, mostly at the time of creation. Which means that it does not keep track of changing variables like these counters. After reloading; these counters restart at 0.

possible OPS 1.x solutions

I don't know what the timeline from @dwhswenson is for a 2.0 release, so none of these fixes might be implemented

  1. We can add a restart_at_step() for the external engine (like we have doen for SpringShooting) and add this to the examples/docs

  2. let Simulations carry references to the engine objects and add the above logic it to the restart_at_step

  3. We just add the above dirty solution to the documentation

proposed OPS 2.0 solution

We act on the comment in FileNameSetter:

"""Just use numbers, as we did previously.

This is the default for compatibility reasons, but it not recommended.
Generally, we recommend using :class:.RandomString.
"""

and update the default on this line to RandomStringFilenames with a DeprecationWarning in OPS 1.x

@dwhswenson any thoughts on this?

@dwhswenson
Copy link
Member

We can add a restart_at_step() for the external engine (like we have doen for SpringShooting) and add this to the examples/docs

I think this is the best solution here. The best way for OPS 2.0 might be for simulation objects to implement a restart_at_step method, then use the serialization tools in SimStore to identify all simulation objects, and for simulation.restart_at_step to call that for all objects that implement it (this also solves it for SpringShooting).

@jjfsteuer : As @sroet mentions in his suggestion for 2.0, 2.0 will default to using random filenames instead of sequential numbers. You can already use this functionality, and I would recommend it. It should solve the problem you're having. See #933 for details and some other reasons that it was originally implemented. Of course, this doesn't change the data you've already created, but for future data, it's easier.

(Note to self -- not sure that the 2.0 branch actually has that default yet, but it should be possible to do.)

@sroet
Copy link
Member

sroet commented Jan 27, 2022

(Note to self -- not sure that the 2.0 branch actually has that default yet, but it should be possible to do.)

It does not, hence my proposal for a deprecation warning + 2.0 update

@jjfsteuer
Copy link
Author

Thank you so much for the quick response!

@sroet adjusting the filename_setter.count is a nice workaround and works great for me.

@dwhswenson thanks for the #933 suggestion, I will update my code accordingly. I'm currently still trying out a lot of things, so the existing data is not that substantial.

@gyorgy-hantal
Copy link

Hello,

Two months ago I raised an issue (#1107, merged already with this one) with the same problem. Using the quick and dirty trick suggested by @sroet solved the problem of restarting from a given step.
I have however another issue: OPS does not seem to append the database file: it stays untouched and no database is written. I use the new database and run the following code (perhaps not all imports are needed):

import openpathsampling as paths
import numpy as np
import mdtraj as md
import os
from openpathsampling.engines import gromacs as ops_gmx
from openpathsampling.experimental.storage import monkey_patch_all
from openpathsampling.experimental.storage import Storage
paths = monkey_patch_all(paths)
paths.InterfaceSet.simstore = True

storage = Storage("retis1.db", mode='a')
retis_calc = storage.pathsimulators[0]
engine = storage.engines[0]

engine.filename_setter.count = 20 # number of last segment written
retis_calc.restart_at_step(storage.steps[-1])
retis_calc.run(50000)

Do you have an idea of what can go wrong?
Thank you!

@sroet
Copy link
Member

sroet commented May 9, 2022

If I remember correctly, restoring a pathsimulator does not actually re-attach the storage object.
Do you mind printing

print(retis_calc.storage)

If that one is None (or does not print anything), you should probably add the line

retis_calc.storage = storage

before running the calculation

@gyorgy-hantal
Copy link

It indeed resulted in None; and adding this

retis_calc.storage = storage

did solve the problem! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants