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

Lammps gjf #1673

Merged
merged 54 commits into from Oct 4, 2019

Conversation

@charlessievers
Copy link
Collaborator

charlessievers commented Sep 11, 2019

Summary

This updated implementation of the {gjf} thermostat includes the choice between
outputting either the on-site {vfull} or half-step {vhalf} velocity. The on-site
velocity has been updated to be the GJF on-site velocity, and the half-step
velocity is the statistically correct 2GJ velocity. The implementation
also takes advantage of Gaussian distributed random numbers in order to achieve
correct fluctuations.

Related Issues

Fixed onsite velocity when the Langevin GJF keyword is set.

Author(s)

Charles A. Sievers, UC Davis Department of Chemistry, charliesievers@cox.net
Niels Gronbech-Jensen, UC Davis Departments of Mechanical Engineering and Mathematics, ngjensen@ucdavis.edu

Licensing

By submitting this pull request, I agree, that my contribution will be included in LAMMPS and redistributed under either the GNU General Public License version 2 (GPL v2) or the GNU Lesser General Public License version 2.1 (LGPL v2.1).

Backward Compatibility

The GJF keyword for the Langevin fix no longer supports yes as an option. The yes option raises an error with a description to notify the user. The description is, "Fix langevin gjf yes is outdated, please use vhalf or vfull" The GJF keyword does not support a t_period equal to the timestep divided by two. The description for this error reads as, "Fix langevin gjf cannot have t_period equal to dt/2 at the start"

Implementation Notes

The relevant changes are the respa error, updated onsite velocity, included 2GJ velocity, and included gaussian noise for only GJF. I also updated the new GJF implementation to be compatible with the zero, tbias, and tally options. Numerous simulation results confirm statistical expectations for both configurational and kinetic properties, exemplified by the attached figures (GJF U {vhalf} GJF V {vfull}). No other features in LAMMPS are affected by these changes.

Post Submission Checklist

Please check the fields below as they are completed after the pull request has been submitted. Delete lines that don't apply

  • The feature or features in this pull request is complete
  • Licensing information is complete
  • Corresponding author information is complete
  • The source code follows the LAMMPS formatting guidelines
  • Suitable new documentation files and/or updates to the existing docs are included
  • The added/updated documentation is integrated and tested with the documentation build system
  • The feature has been verified to work with the conventional build system
  • The feature has been verified to work with the CMake based build system
  • A package specific README file has been included or updated
  • One or more example input decks are included

Further Information, Files, and Links
_
argon_kinetic_energy_fluctuations.pdf
argon_kinetic_energy.pdf
argon_potential_energy_fluctuations.pdf
argon_potential_energy.pdf
guaiacol_kinetic_energy_fluctuations.pdf
guaiacol_kinetic_energy.pdf
guaiacol_potential_energy_fluctuations.pdf
guaiacol_potential_energy.pdf

Literature concerning these implementations:
https://doi.org/10.1080/00268976.2012.760055
https://doi.org/10.1080/00268976.2019.1570369
https://doi.org/10.1080/00268976.2019.1662506
_

charlie sievers added 8 commits Sep 17, 2019
charlie sievers charlie sievers
charlie sievers charlie sievers
charlie sievers charlie sievers
charlie sievers charlie sievers
@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Sep 25, 2019

assigning this PR to @athomps for a first detailed appraisal. i am getting a bit concerned that we are overloading fix langevin with to many variants and functionality. perhaps we should consider splitting it into multiple derived classes. there are already multiple langevin variants in other fixes (with and without time integration included).

@@ -217,6 +217,10 @@ the particles. As described below, this energy can then be printed
out or added to the potential energy of the system to monitor energy

This comment has been minimized.

Copy link
@athomps

athomps Sep 26, 2019

Contributor

What about modifying the syntax summary:

{gjf} value = {vfull} or {vhalf}
{vfull} = ...
{vhalf} = ...

This comment has been minimized.

Copy link
@charlessievers

charlessievers Sep 26, 2019

Author Collaborator

Hello @athomps, thank you for catching that.

velocity distribution, such as the velocity auto-correlation function
(VACF). In this example, the velocity distribution at dt = 2.5fs
generates an average temperature of 220 K, instead of 300 K.
An example of a reason why to use the {gjf} keyword is the freedom to take a larger time step,

This comment has been minimized.

Copy link
@athomps

athomps Sep 26, 2019

Contributor

This whole block of text is redundant and/or beyond the scope of LAMMPS documentation. It should be deleted:

An example of a reason why to use the {gjf} keyword is the freedom to take a larger time step,
up to the stability limit, while maintaining robust statistics. It is crucial to
recall that while the equilibrium statistics is appropriately sampled, the correct dynamics
of the trajectories may not be for large time steps, as is the case for all thermostats.
All thermostats provide good statistics and dynamics for small time steps.
The 2GJ half-step velocity {vhalf} samples the correct velocity distribution for the {gjf} trajectory.

This updated implementation of the {gjf} thermostat includes the choice between
outputting either the on-site {vfull} or half-step {vhalf} velocity. The on-site
velocity has been updated to be the GJF on-site velocity, and the half-step
velocity is the statistically correct 2GJ velocity. The implementation
also takes advantage of Gaussian distributed random numbers in order to achieve
correct fluctuations.

This comment has been minimized.

Copy link
@charlessievers

charlessievers Sep 26, 2019

Author Collaborator

It has been removed.

else if (strcmp(arg[iarg+1],"yes") == 0) gjfflag = 1;
if (strcmp(arg[iarg+1],"no") == 0) {gjfflag = 0; osflag = 0;}
else if (strcmp(arg[iarg+1],"yes") == 0)
error->all(FLERR,"Fix langevin gjf yes is outdated, please use vhalf or vfull");

This comment has been minimized.

Copy link
@athomps

athomps Sep 26, 2019

Contributor

I think this special error message is unnecessary. Anything other than vfull or vhalf should give the standard "Illegal fix langevin command" message. Also, it is a good idea to consistently refer to vfull and vhalf in that order in the code, since that is the order in the documentation. You should also try to use standard LAMMPS whitespace style:
else if (strcmp(arg[iarg+1],"vhalf") == 0) {
gjfflag = 1;
osflag = 1;
}

This is not yet codified anywhere, but we know it when we (don't) see it.

Copy link
Contributor

athomps left a comment

Nice job! It is good to go, once my comments are addressed.

@akohlmey akohlmey assigned akohlmey and unassigned athomps Sep 26, 2019
@charlessievers

This comment has been minimized.

Copy link
Collaborator Author

charlessievers commented Oct 2, 2019

Hello @akohlmey what is the next step for this pull request? Is there anything I should be doing?

@akohlmey akohlmey requested review from athomps and akohlmey Oct 3, 2019
@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Oct 3, 2019

Hello @akohlmey what is the next step for this pull request? Is there anything I should be doing?

i think the ball is in the court of the LAMMPS developers right now. i am particularly waiting for @athomps to let me know if he is satisfied with your latest changes. will have to check on top of that myself on the usual requirements for merging. my time is limited right now, so processing and merging of pull requests will slow down over the next 3 weeks. i am also waiting on folks working through older pull requests that have been assigned to them, but got left behind.

@athomps
athomps approved these changes Oct 3, 2019
Copy link
Contributor

athomps left a comment

This all looks good to me.

@akohlmey akohlmey merged commit ed5678d into lammps:master Oct 4, 2019
8 checks passed
8 checks passed
lammps/pull-requests/build-docs-pr head run ended
Details
lammps/pull-requests/cmake/cmake-kokkos-cuda-pr head run ended
Details
lammps/pull-requests/cmake/cmake-kokkos-omp-pr head run ended
Details
lammps/pull-requests/cmake/cmake-serial-pr head run ended
Details
lammps/pull-requests/kokkos-omp-pr head run ended
Details
lammps/pull-requests/openmpi-pr head run ended
Details
lammps/pull-requests/serial-pr head run ended
Details
lammps/pull-requests/shlib-pr head run ended
Details
@charlessievers charlessievers referenced this pull request Oct 15, 2019
1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.