-
Notifications
You must be signed in to change notification settings - Fork 22
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] Full unit-awareness when parsing LAMMPS outputs with drivers #902
Labels
lammps
Relating to LAMMPS
Comments
timbernat
changed the title
Full unit-awareness when parsing LAMMPS outputs with drivers
[LAMMPS] Full unit-awareness when parsing LAMMPS outputs with drivers
Feb 14, 2024
Quick thoughts now, might add more later - happy to look at a PR for any of this!
|
|
One of the text files here controls which are exposed by our registry, which is mostly Pint's default with macro-scale units stripped out. Adding units to the registry is nearly free, so anything that might be useful for your work (eV, etc.) is probably useful for others and can be added |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
In the current implementation of get_lammps_energies, log file parsing is hard-coded to energies of kJ/mol and has no awareness of the various unit styles that LAMMPS supports. Currently MDConfig.write_lammps_input() only supports the "real" unit style, so this is a fair assumption for the current implementation, but is not robust in the not-unreasonable case where one is provided/generates an input file which deviates from the MDConfig hard-coded defaults. Dynamic awareness of the unit system being used would remove a source or error (and a lot of guesswork) when comparing energies between platforms, as well as other unit-dependent quantites (e.g. unit cell sizes, velocities, etc)
I've already implemented both a comprehensive reference of LAMMPS unit styles (in OpenMM units) and functionality for detecting/applying these settings from input files. Would these codes (in some modified form) be amenable to a PR or future feature?
The text was updated successfully, but these errors were encountered: