Add MLIP compatibility module#371
Open
jan-janssen wants to merge 15 commits into
Open
Conversation
Documents the plan to re-implement pyiron_atomistics' legacy LammpsMlip job class as standalone, stateless functions in src/lammpsparser/compatibility/mlip.py, without any pyiron dependencies. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
First piece of the standalone MLIP compatibility module: generating the mlip.ini control file for pair_style mlip (including active learning mode) and checking error.out for the MLIP breaking-threshold condition. Ports LammpsMlip.write_input/enable_active_learning and LammpsMlip.convergence_check from pyiron_atomistics without any pyiron dependency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1. Remove unused import: `from typing import Optional` 2. Fix docstring/implementation mismatch in `write_mlip_input_file`: wrap the return path in `os.path.abspath()` to ensure it truly returns an absolute path as documented, not just a relative path when the input directory is relative. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Implements a from-scratch parser for the plain-text MTP .cfg format (BEGIN_CFG/Size/Supercell/AtomData/Energy/PlusStress/Feature/END_CFG), replacing the pyiron_potentialfit.mlip.cfgs.loadcfgs dependency the legacy LammpsMlip job relied on. Stress is parsed directly into ASE's Voigt order (xx, yy, zz, yz, xz, xy) by column name, so the parser is robust to header reordering. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Builds ase.atoms.Atoms from parsed MLIP .cfg configurations, mapping integer type indices to chemical symbols via the potential's species list and attaching forces/energy/stress/mv_grade as plain arrays/info entries (no SinglePointCalculator). Replaces LammpsMlip.collect_output()'s .cfg-parsing loop, the selected_structures property, and the StructureStorage dependency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Makes write_mlip_input_file, check_mlip_convergence, load_mlip_cfgs, and get_mlip_selected_structures importable directly from lammpsparser, matching how the other compatibility functions (calc_md, lammps_file_interface_function, etc.) are already re-exported. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
Contributor
📝 WalkthroughWalkthroughAdds standalone MLIP compatibility utilities for generating input files, checking convergence, parsing selected configurations, creating ASE structures, exposing package-level helpers, and validating behavior with CFG fixtures. It also regenerates package version metadata. ChangesMLIP compatibility
Version metadata
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant get_mlip_selected_structures
participant load_mlip_cfgs
participant ASEAtoms
Caller->>get_mlip_selected_structures: provide selected.cfg and species
get_mlip_selected_structures->>load_mlip_cfgs: parse CFG records
load_mlip_cfgs-->>get_mlip_selected_structures: return configurations
get_mlip_selected_structures->>ASEAtoms: create atoms and attach metadata
ASEAtoms-->>Caller: return selected structures
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #371 +/- ##
==========================================
+ Coverage 98.77% 98.87% +0.10%
==========================================
Files 13 14 +1
Lines 1223 1337 +114
==========================================
+ Hits 1208 1322 +114
Misses 15 15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/superpowers/specs/2026-07-13-mlip-compatibility-design.md (1)
32-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFenced code blocks in documentation lack language specifiers (markdownlint MD040). All three code blocks showing
mlip.inior.cfgcontent are rendered without syntax highlighting and trigger MD040. Add a language tag to each fence.
docs/superpowers/specs/2026-07-13-mlip-compatibility-design.md#L32-L48: addinito the twomlip.iniexample code blocks (lines 32 and 38).docs/superpowers/plans/2026-07-13-mlip-compatibility.md#L291-L319: addtextorinito theselected.cfgfixture code block.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-07-13-mlip-compatibility-design.md` around lines 32 - 48, Add the ini language specifier to both mlip.ini fenced code blocks in docs/superpowers/specs/2026-07-13-mlip-compatibility-design.md (lines 32-48). Add a text or ini language specifier to the selected.cfg fixture fence in docs/superpowers/plans/2026-07-13-mlip-compatibility.md (lines 291-319); no other content changes are needed.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/superpowers/specs/2026-07-13-mlip-compatibility-design.md`:
- Around line 32-48: Add the ini language specifier to both mlip.ini fenced code
blocks in docs/superpowers/specs/2026-07-13-mlip-compatibility-design.md (lines
32-48). Add a text or ini language specifier to the selected.cfg fixture fence
in docs/superpowers/plans/2026-07-13-mlip-compatibility.md (lines 291-319); no
other content changes are needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b68365ce-2426-4eb1-b964-2cd72bf1e798
📒 Files selected for processing (7)
docs/superpowers/plans/2026-07-13-mlip-compatibility.mddocs/superpowers/specs/2026-07-13-mlip-compatibility-design.mdsrc/lammpsparser/__init__.pysrc/lammpsparser/compatibility/mlip.pytests/static/mlip/selected.cfgtests/test_compatibility_mlip.pytests/test_top_level_exports.py
Member
Author
|
@copilot extend unit tests to cover newly added code with unit tests |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lammpsparser/_version.py`:
- Around line 5-12: Resolve Ruff RUF022 for the generated version module by
updating the generator or template that emits __all__ so its entries are sorted,
or configure Ruff to exclude generated version files from this rule. Do not edit
src/lammpsparser/_version.py directly; preserve the generated module’s existing
exports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 95cfa369-9603-4f68-9476-ac1780e70c99
📒 Files selected for processing (1)
src/lammpsparser/_version.py
Comment on lines
+5
to
+12
| __all__ = [ | ||
| "__version__", | ||
| "__version_tuple__", | ||
| "version", | ||
| "version_tuple", | ||
| "__commit_id__", | ||
| "commit_id", | ||
| ] |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Resolve Ruff RUF022 for the generated module.
__all__ is not sorted, which can fail Ruff checks. Since this file is generated, fix the generator/template ordering or exclude generated version files from RUF022 rather than editing this artifact manually.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 5-12: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lammpsparser/_version.py` around lines 5 - 12, Resolve Ruff RUF022 for
the generated version module by updating the generator or template that emits
__all__ so its entries are sorted, or configure Ruff to exclude generated
version files from this rule. Do not edit src/lammpsparser/_version.py directly;
preserve the generated module’s existing exports.
Source: Linters/SAST tools
Contributor
Done. Extended New coverage added:
Two static |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/test_compatibility_mlip.py (2)
217-227: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDiagonal cell can't distinguish matrix dot-product from elementwise scaling.
direct_coords.cfguses a diagonal cell (4,4,4). Direct→cartesian conversion is properly a matrix product (cartesian = fractional @ cell); for a diagonal cell this is numerically indistinguishable from a buggy elementwise multiply by the diagonal. A non-orthogonal (triclinic) fixture cell is needed to actually exercise/verify the matrix multiplication path.See consolidated comment for the paired fixture-file concern.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_compatibility_mlip.py` around lines 217 - 227, Update test_direct_coordinates_converted_to_cartesian to use a non-orthogonal triclinic cell and matching fractional coordinates whose expected Cartesian positions require a matrix product. Update the direct_coords.cfg fixture accordingly while preserving the existing type, force, and energy assertions, so elementwise scaling cannot satisfy the test.
235-311: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated temp-file write/unlink boilerplate across 5 tests.
Each negative-parsing test duplicates the
NamedTemporaryFile(...)/try/finally: os.unlink(...)pattern. Extract a small helper (e.g., a context manager orsetUp-level utility that writes content to a temp.cfgand yields the path) to reduce duplication.Example helper
from contextlib import contextmanager `@contextmanager` def _temp_cfg(content): with tempfile.NamedTemporaryFile(mode="w", suffix=".cfg", delete=False) as tmp: tmp.write(content) tmp_path = tmp.name try: yield tmp_path finally: os.unlink(tmp_path)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_compatibility_mlip.py` around lines 235 - 311, Extract the repeated temporary CFG file setup and cleanup from the five tests into a small shared helper, such as a context manager near the test class. Update test_end_cfg_without_begin_cfg_raises, test_missing_supercell_raises, test_missing_atomdata_raises, test_atomdata_before_size_raises, and test_empty_file_returns_empty_list to use the helper while preserving their existing contents and assertions.tests/static/mlip/direct_coords.cfg (1)
1-13: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDiagonal cell limits coverage of the direct→cartesian conversion path.
See consolidated comment (paired with
tests/test_compatibility_mlip.py#L217-227) for details and suggested fixture change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/static/mlip/direct_coords.cfg` around lines 1 - 13, Update the Supercell fixture in direct_coords.cfg from a purely diagonal cell to a valid non-diagonal cell, preserving the existing atom count, direct coordinates, forces, and energy unless the paired compatibility test requires corresponding expected Cartesian values to change. Ensure the fixture exercises off-diagonal direct-to-Cartesian conversion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/static/mlip/direct_coords.cfg`:
- Around line 1-13: Update the Supercell fixture in direct_coords.cfg from a
purely diagonal cell to a valid non-diagonal cell, preserving the existing atom
count, direct coordinates, forces, and energy unless the paired compatibility
test requires corresponding expected Cartesian values to change. Ensure the
fixture exercises off-diagonal direct-to-Cartesian conversion.
In `@tests/test_compatibility_mlip.py`:
- Around line 217-227: Update test_direct_coordinates_converted_to_cartesian to
use a non-orthogonal triclinic cell and matching fractional coordinates whose
expected Cartesian positions require a matrix product. Update the
direct_coords.cfg fixture accordingly while preserving the existing type, force,
and energy assertions, so elementwise scaling cannot satisfy the test.
- Around line 235-311: Extract the repeated temporary CFG file setup and cleanup
from the five tests into a small shared helper, such as a context manager near
the test class. Update test_end_cfg_without_begin_cfg_raises,
test_missing_supercell_raises, test_missing_atomdata_raises,
test_atomdata_before_size_raises, and test_empty_file_returns_empty_list to use
the helper while preserving their existing contents and assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cba3d5e2-d93e-4990-aa6a-9fe316802e76
📒 Files selected for processing (4)
src/lammpsparser/_version.pytests/static/mlip/direct_coords.cfgtests/static/mlip/reordered_stress.cfgtests/test_compatibility_mlip.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lammpsparser/_version.py
Updated versioning method to use setuptools-scm and simplified version tuple definitions.
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.
Summary
LammpsMlipjob class (MTP/MLIP interatomic potential support) as a standalone, stateless modulesrc/lammpsparser/compatibility/mlip.py, with zero pyiron dependencies.write_mlip_input_file,check_mlip_convergence,MlipConfiguration,load_mlip_cfgs(a from-scratch parser for the plain-text MTP.cfgformat), andget_mlip_selected_structures.MlipConfiguration) are re-exported from the top-levellammpsparserpackage.docs/superpowers/specs/2026-07-13-mlip-compatibility-design.md; implementation plan indocs/superpowers/plans/2026-07-13-mlip-compatibility.md.Test plan
python -m unittest discover tests— 182 tests, all passing (17 pre-existing environment-specific skips)ValueErrorpaths,direct_x/y/zcoordinate branch, reorderedPlusStresscomponents, customerror_file_name, custom active-learning parameters,MlipConfigurationdataclass construction, andIndexErroron species/type mismatch🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation
Summary by CodeRabbit
New Features
Documentation
Tests