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

Compute pace update #3869

Merged
merged 48 commits into from Dec 12, 2023
Merged

Compute pace update #3869

merged 48 commits into from Dec 12, 2023

Conversation

jmgoff
Copy link
Collaborator

@jmgoff jmgoff commented Jul 24, 2023

Summary

With the growing number of atomic cluster expansion (ACE) users, a LAMMPS compute is needed at this time that can evaluate the descriptors for various methods and workflows such as FitSNAP and ML-IAP. This update features a new LAMMPS compute, compute pace, that evaluates ACE descriptors. The style mirrors the compute for SNAP descriptors, compute snap, and provides users a familiar method to evaluate ACE descriptors in LAMMPS for data analysis, for model training, etc.

Related Issue(s)

N/A

Author(s)
(Alphabetical)

Aidan Thompson (Sandia National Laboratories)
Andrew Rohskopf (Sandia National Laboratories)
James Goff (Sandia National Laboratories) ( goffjimmy1@gmail.com )
Yury Lysogorskiy (Ruhr University for lammps-user-pace backend)

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
No problems with old inputs.

Implementation Notes

The calculation of ACE descriptors and descriptor gradients were validated numerically against those from the existing pair_style pace in LAMMPS. They have also been validated against other ACE software that is not in LAMMPS.

This initial implementation of compute pace requires minor changes to the lammps-user-pace library that is downloaded whenever the ML-PACE D flag is turned on for compilation. The required changes to the lammps-user-pace library are essentially just allocating arrays to store ACE descriptors and gradients. Until such changes are made in the default lamps-user-pace repo at ICAMS, some errors may be thrown unless a separate fork of the lammps-user-pace library is used. Future updates and integration with the (external ACE compute)[https://github.com/ICAMS/python-ace/issues/14] developed by @yury-lysogorskiy at ICAMS would resolve this as well.

This first compute pace is here to provide users with the ACE descriptor compute capabilities immediately and serves as the base framework to integrate more standardized descriptor + gradient calculators in the future.

Post Submission Checklist

  • 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
  • Suitable tests have been added to the unittest tree.
  • A package specific README file has been included or updated
  • One or more example input decks are included

Further Information, Files, and Links

Link to the original ICAMS PACE libraries and those including an updated descriptor + gradient calculator: https://github.com/ICAMS/lammps-user-pace and ICAMS/python-ace#14
Link to modified lammps-user-pace: https://github.com/jmgoff/lammps-user-pace-1

jmgoff and others added 27 commits September 28, 2022 10:12
Update the ML-PACE code to include recent updates from lammps-user-pace
including gamma pair fix.
…into compute-pace

Updated with new PRs from Yuri
Copy link
Member

@akohlmey akohlmey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a comment for a required change to the compute_pace.h file.
This code also seems to be dependent on a different version of the ACE library since our CI testing fails to compile with errors indicating missing definitions. See, e.g. https://ci.lammps.org/job/dev/job/pull_requests/job/fedora/job/cmake_kokkos_mpi_openmp_clang_shared/7591/console
Thus the corresponding build files need to be updated to download a suitable version.

src/ML-PACE/compute_pace.h Outdated Show resolved Hide resolved
@jmgoff jmgoff marked this pull request as ready for review October 20, 2023 00:22
@akohlmey akohlmey self-assigned this Nov 15, 2023
Copy link
Contributor

@stanmoore1 stanmoore1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good, but needs a little clean up as I mentioned in some of the comments.

examples/PACKAGES/pace/compute/README.md Outdated Show resolved Hide resolved
examples/test.txt Outdated Show resolved Hide resolved
src/ML-PACE/compute_pace.cpp Outdated Show resolved Hide resolved
src/ML-PACE/compute_pace.cpp Outdated Show resolved Hide resolved
@akohlmey
Copy link
Member

akohlmey commented Dec 5, 2023

@jmgoff I've merged with upstream and implemented requested changes (and a couple of mostly cosmetic updates, too).

Please check it out and let us know if this is now a final version or still waiting for additional modifications.

@jmgoff
Copy link
Collaborator Author

jmgoff commented Dec 12, 2023

@akohlmey Thank you for making those changes and updates requested by @stanmoore1 (and for your patience with my slow updates on this)! It looks good to me and is the final version for this PR.

@akohlmey akohlmey merged commit c33d950 into lammps:develop Dec 12, 2023
4 checks passed
@jmgoff jmgoff mentioned this pull request Dec 13, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants