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

Fix several issues preventing reading state from LAMMPS restart file #579

Merged
merged 9 commits into from
Sep 7, 2023

Conversation

giacomofiorin
Copy link
Member

@giacomofiorin giacomofiorin commented Sep 6, 2023

With both NAMD and LAMMPS now allowing to first initialize Colvars after a simulation has begun (initial step > 0), it's getting harder to get everything right for moving restraints that require knowledge of the step when they were first defined (see #486).

This PR fixes this behavior by recording that number as firstStep and embedding it in the state file.

For LAMMPS specifically, this now works both with a Colvars state file provided via the input keyword or by the LAMMPS binary restart file.

@giacomofiorin giacomofiorin added LAMMPS bugfix To be used only in PRs labels Sep 6, 2023
@giacomofiorin giacomofiorin force-pushed the fix-lammps-restart branch 2 times, most recently from 895cc49 to ddea477 Compare September 6, 2023 20:47
@giacomofiorin giacomofiorin marked this pull request as ready for review September 7, 2023 00:09
@jhenin
Copy link
Member

jhenin commented Sep 7, 2023

Looks good. Why is the new function delete_input_stream necessary? What happens with close_input_stream?

@giacomofiorin
Copy link
Member Author

Looks good. Why is the new function delete_input_stream necessary? What happens with close_input_stream?

It just rewinds the string stream to allow future reads (as when the files are read as strings embedded in a TPR file), but then the condition below remains true, and the input state string keeps being loaded every time setup_input() is called:

colvars/src/colvarmodule.cpp

Lines 1313 to 1320 in 0a997fc

if (proxy->input_stream_exists("input state string")) {
cvm::log(cvm::line_marker);
cvm::log("Loading state from string.\n");
read_restart(proxy->input_stream("input state string"));
cvm::log(cvm::line_marker);
proxy->close_input_stream("input state string");
}

The above is clearly an issue that needs to be fixed, either by adding a new function (as in this case) or a separate flag that is activated once and deactivated after the state is read. This is where a second set of eyes is most helpful ;-)

@jhenin jhenin self-requested a review September 7, 2023 16:11
@giacomofiorin giacomofiorin merged commit 7ea7712 into master Sep 7, 2023
29 checks passed
@giacomofiorin giacomofiorin deleted the fix-lammps-restart branch September 14, 2023 14:22
@giacomofiorin giacomofiorin mentioned this pull request Aug 5, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix To be used only in PRs LAMMPS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants