-
Notifications
You must be signed in to change notification settings - Fork 6
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 converter fix #430
Lammps converter fix #430
Conversation
…to lammps-converter-fix
…to lammps-converter-fix
…to lammps-converter-fix
Looks good. How do we convert lammps h5md and other files from the GUI? or is this not supported? |
I just used the combo box in the converter panel to specify that the input file is h5md or xyz. You are right that we should probably change the filters in the file dialogs. For some reason my LAMMPS files never have the extensions that the converter expects, so I always have to switch to I will check if there is any official LAMMPS convention for file name extensions. Maybe I have been naming my files wrong, and MDANSE converter is right. |
Looking at the examples in the LAMMPS documentation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Description of work
The existing LAMMPS converter works correctly only for a small number of specific cases. LAMMPS gives the user multiple options of the physical unit system output file format.
The converter has been extended to allow the user to pick the unit system, and supports three trajectory formats: LAMMPS custom format (this has been considered to be the ".lammps" format so far), XYZ and LAMMPS implementation of H5MD.
In principle, another change will be necessary in the future to account for different
atom_style
options, which will change the format of the structure file (known in MDANSE as the 'config' file).closes #421
Fixes
Added multiple readers to the LAMMPS converter.
Added the unit system selector.
Included additional trajectories for unit testing, together with the input files used to generate them.
Added new unit tests for the LAMMPS converter.
To test
All tests must pass.
Also, please try to convert a LAMMPS trajectory using the protos branch and again using this branch. The results should be the same.
Optionally, please try to manually convert the new trajectory files from unit test data and check that they all result in the same output.