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

Adding test examples for the SPIN package and correcting some bugs #1771

Merged
merged 22 commits into from Dec 1, 2019

Conversation

@julient31
Copy link
Collaborator

julient31 commented Nov 15, 2019

Summary

This PR contains three main modifications:

  • setting three first test examples for the SPIN package (in examples/SPIN/test_problems) with an associated README file.
  • improvements of the SPIN package's documentation (mainly fix_langevin_spin.txt, pair_spin_exchange.txt, and compute_spin.txt).
  • correction of some bugs (mainly found and solved with the benchmark examples), for example in fix precession/spin.

Related Issues

None

Author(s)

Julien Tranchida (jtranch@sandia.gov)

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

Implementation Notes

Correctness of the changes is checked with the newly setup benchmark examples.

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

julient31 added 10 commits Oct 28, 2019
- corrected issue in Zeeman precession
- improved documentation of Zeeman interaction
- commit changes examples
- removed all tabs from spin package
- comments in precession
- modified precession and Langevin/spin
- addind 4 first benchmark examples (in examples/SPIN/benchmark)
- corrected typo in examples (in dump commands)
- modified documentation spin (compute and exchange)
- modified compute spin for Ts
- add README file to the benchmark examples repo
- removed comments from src/SPIN files
- adding doc exceptions to false-positives
@julient31 julient31 requested a review from rbberger as a code owner Nov 15, 2019
@athomps athomps self-assigned this Nov 19, 2019
@athomps athomps requested review from athomps and akohlmey and removed request for rbberger Nov 19, 2019
Copy link
Contributor

athomps left a comment

I went over this with Julien and it all looks good to me. The only change to the regression testing log files is the in the syntax of the dump command.

@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Nov 19, 2019

@julient31 I would not talk about "benchmark" in this context here, but about "validation". The former term is too often used in the context of LAMMPS to describe how fast a calculation is or how well it parallelizes rather than how accurate it is. Also there are references and files/folders using "benchmarck" instead of "benchmark". I would be ok with the term "benchmark" if @sjplimp, @athomps, @stanmoore1 can assure me that there would be no confusion, but at least the typo needs to be resolved. ;-)

@athomps

This comment has been minimized.

Copy link
Contributor

athomps commented Nov 19, 2019

@julient31 @akohlmey I agree that the term benchmark is confusing because it collides with the same word used in the context of LAMMPS performance. More generally, benchmarks are things that are measures of performance, capacity, distance traveled i.e. magnitudes. The examples provided appear to be measures of correctness rather than performance. I suggest replacing benchmarck with test. It's less exciting, but it is an accurate description and contains fewer characters.

julient31 added 2 commits Nov 19, 2019
- modified named of tests, from benchmark to validation
- renaming validation directory
@julient31

This comment has been minimized.

Copy link
Collaborator Author

julient31 commented Nov 19, 2019

@akohlmey @athomps Yes good point: benchmark was not very well chosen. I renamed the directory 'test_problems', and the sub-directories will be validating different calculations (validation_damped_exchange, validation_damped_precession, ...).
@akohlmey , Axel, as I corrected the dump commands, would you like/need me to regenerate the log files in the other examples? (for example log.30Apr19.spin.iron.g++.1, ...).

akohlmey added 4 commits Nov 19, 2019
@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Nov 19, 2019

@akohlmey , Axel, as I corrected the dump commands, would you like/need me to regenerate the log files in the other examples? (for example log.30Apr19.spin.iron.g++.1, ...).

Yes. Not so much because of the dump command changes, but since it looks you also changed the thermo settings.

@akohlmey akohlmey changed the title Setting up benchmarking examples for the SPIN package, and correcting some bugs Adding test examples for the SPIN package and correcting some bugs Nov 19, 2019
julient31 added 2 commits Nov 19, 2019
- generated fresh log. files in examples
- corrected some of the examples
@julient31

This comment has been minimized.

Copy link
Collaborator Author

julient31 commented Nov 19, 2019

@akohlmey I generated fresh log files corresponding to the current results of the code / input examples.
Also Axel, I left in the examples/SPIN directory a shell script (run_spin_examples.sh) that is running all the examples and generating the associated log files. It's a little bit 'messy', but I would like to keep it there to regenerate fresh log files each time I modify the SPIN package or the examples.
Is it fine to leave it there? Otherwise, let me know, I'll it remove right away.
Thanks a lot!

julient31 added 2 commits Nov 22, 2019
- improved figure in precession/spin documentation
- corrected other pair/spin interactions
- correcting issue in src/SPIN/atom_vec_spin.cpp (inconsistency packing/unpacking hybrid)
- rerunning all examples with corrections of former commit
@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Nov 25, 2019

@athomps for me to merge it, you need to assign the PR to me. Thanks.

@athomps athomps assigned akohlmey and unassigned athomps Nov 25, 2019
@rbberger rbberger assigned rbberger and unassigned akohlmey Nov 26, 2019
julient31 and others added 2 commits Nov 27, 2019
- correcting 'neighbours' in doc
- modifying README is src/SPIN
@rbberger

This comment has been minimized.

Copy link
Member

rbberger commented Nov 27, 2019

@julient31 I've updated the docs to be RST only. Please review my changes.

@rbberger rbberger assigned julient31 and unassigned rbberger Nov 27, 2019
@julient31

This comment has been minimized.

Copy link
Collaborator Author

julient31 commented Nov 27, 2019

@rbberger Thanks for converting the files Richard! I'll write/update directly .rst files next time.
I checked them and that all look good to me!

@julient31 julient31 removed their assignment Nov 28, 2019
@rbberger rbberger self-assigned this Dec 1, 2019
@rbberger rbberger merged commit 3611a75 into lammps:master Dec 1, 2019
6 checks passed
6 checks passed
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
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.