Update tutorial/docs, improve C++ error handling, and fix Kokkos types - #451
Open
DavidW99 wants to merge 16 commits into
Open
Update tutorial/docs, improve C++ error handling, and fix Kokkos types#451DavidW99 wants to merge 16 commits into
DavidW99 wants to merge 16 commits into
Conversation
Removed unfix command for 'nve' to be compatible with ase==3.27.0 lammps calculator logic
using += when adding command strings to a list will breaks the string into single char
Move skip logic inside all_gp fixture body using pytest.skip()
ASE uses a string for 'velocity'
There was a problem hiding this comment.
Pull request overview
This pull request enhances the FLARE codebase with comprehensive improvements across documentation, C++ LAMMPS plugins, Kokkos implementations, and Python code. The changes focus on updating installation requirements, improving error handling in file I/O operations, modernizing Kokkos type definitions, and fixing several bugs in Python code.
Changes:
- Updated installation documentation to use Python 3.10 and GCC 11, and removed the development branch requirement
- Added comprehensive error handling for file reading operations in LAMMPS C++ plugins with NULL checks on fgets() calls
- Migrated Kokkos code from deprecated float types (X_FLOAT, F_FLOAT, E_FLOAT) to modern types (KK_FLOAT, KK_ACC_FLOAT)
- Fixed Python bugs in LAMMPS calculator related to list/string type handling and corrected sscanf pointer usage in C++ code
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_mgp.py | Moved pytest skip check from decorator to inside fixture body for better clarity |
| tests/test_lammps.py | Corrected velocity parameter type from list to string to match ASE LAMMPS API |
| lammps_plugins/pair_mgp.cpp | Added NULL checks for fgets() calls and fixed sscanf char array pointer usage |
| lammps_plugins/pair_flare.cpp | Added NULL checks for fgets() calls and fixed sscanf char array pointer usage |
| lammps_plugins/compute_flare_std_atom.cpp | Added comprehensive error handling for file reading operations |
| lammps_plugins/kokkos/pair_mgp_kokkos.h | Updated type definitions from F_FLOAT/E_FLOAT to KK_FLOAT/KK_ACC_FLOAT |
| lammps_plugins/kokkos/pair_mgp_kokkos.cpp | Replaced all deprecated Kokkos float types with modern equivalents |
| lammps_plugins/kokkos/pair_flare_kokkos.h | Updated Kokkos type definitions and view types |
| lammps_plugins/kokkos/pair_flare_kokkos.cpp | Migrated to KK_FLOAT and KK_ACC_FLOAT throughout |
| flare/scripts/otf_train.py | Fixed typo: "intialize" → "initialize" |
| flare/md/lammps.py | Fixed model_post list append operations and velocity parameter type |
| docs/source/tutorials/colabs.rst | Updated main tutorial link and description |
| docs/source/installation/install.rst | Updated Python version examples from 3.8 to 3.10 |
| .github/workflows/flare.yml | Updated to GCC 11 and removed ASE patch step |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ASE 3.29 removed the positional energy-unit temperature argument of MaxwellBoltzmannDistribution; the temperature must now be passed as the keyword-only argument temperature_K in Kelvin.
pyclbr's recursive import reads can raise AttributeError (namespace packages) or KeyError (module-cache misses), not just ImportError. Also remove an unused module-level pytest import that broke flare-otf on installs without test dependencies.
YuuuXie
approved these changes
Jul 8, 2026
YuuuXie
left a comment
YuuuXie
left a comment
Collaborator
There was a problem hiding this comment.
I did not read closely into the code changes. But if this is bug fixing, let's merge it soon.
I notice there are a few tests in CI that failed. Once they are fixed then feel free to go ahead merging it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements across the documentation, Python, and C++/Kokkos codebases for FLARE. The changes include updating documentation and installation instructions, improving error handling in the LAMMPS plugin C++ code, and correcting data types for Kokkos compatibility. Additionally, there are minor bug fixes and code cleanups.
Documentation and Installation Updates:
developmentbranch, and updated example paths to reflect Python 3.10. [1] [2] [3] [4]colabs.rst, updating the main FLARE tutorial and improving clarity.LAMMPS Plugin and C++ Improvements:
Kokkos Codebase Corrections:
X_FLOAT/F_FLOATtoKK_FLOAT/KK_ACC_FLOATthroughout the Kokkos pair potential implementation. [1] [2] [3] [4] [5] [6] [7] [8]Python Code Fixes:
model_postandcomputecommands inflare/md/lammps.pyto use lists consistently, preventing string concatenation bugs. Also, corrected the expected type for thevelocityparameter for ASE LAMMPS calculator. [1] [2]flare/scripts/otf_train.py.Workflow Simplification: