Skip to content

feat: add CPU SNAP potential support - #8235

Open
19hello wants to merge 18 commits into
deepmodeling:developfrom
19hello:feature/snap-cpu-develop
Open

19hello wants to merge 18 commits into
deepmodeling:developfrom
19hello:feature/snap-cpu-develop

Conversation

@19hello

@19hello 19hello commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Reminder

  • Read AGENTS.md and docs/developers_guide/agent_governance.md.
  • Added focused unit and CPU case tests.
  • Documented the user interface and exact verification.

Linked issue

Standalone CPU SNAP implementation; no linked issue.

What changed

  • Add CPU SNAP linear, quadratic, and multi-element chemical energy, force, and virial calculations.
  • Support bzeroflag, bnormflag, wselfallflag, chemflag, switchflag, and smooth inner switching (switchinnerflag, sinner, dinner).
  • Convert periodic ghost forces to the same Ry/Bohr units as owned forces.
  • Validate rmin0 against every active pair cutoff; handle the rmin0 boundary consistently; reject unsupported model parameters.
  • Confirm full coefficient and parameter paths to stdout and the running log after successful reads.
  • Document chemical-channel order: coefficients follow the element order in STRU, as in the LAMMPS pair_coeff mapping. Match coefficient element blocks by name; test reordered requests.
  • Reuse the unchanged SNAP environment for analytic force derivatives. Use named derivative seeds and compute the twojmax clamp in the constructor body.
  • Move cell-level energy, force, and virial accumulation into module_snap::CellAccumulator, used by both MDCell and UnitCell. The solver retains neighbor search, MPI reduction, and unit conversion at the ABACUS boundary.
  • Add a three-atom non-collinear angular and quadratic regression checked against LAMMPS, a virial strain check, and a UnitCell single-point regression.

Use

esolver_type    snap
pot_file        snap.coeff
snap_param_file snap.param

SNAP model parameters and chemical-channel ordering are documented in docs/advanced/md.md.

Verification

Commands run:

cmake --build /tmp/abacus-snap-check2 -j2
ctest --test-dir /tmp/abacus-snap-check2 --output-on-failure -R 'MODULE_MD_SNAP_|MODULE_IO_(read_input_serial|read_item_serial)'
OMP_NUM_THREADS=1 bash ../integrate/Autotest.sh -a /tmp/abacus-snap-check2/abacus_pw_ser -n 1 -r '10[23456]_SNAP_'
python3 tools/03_code_analysis/agent_governance_check.py --base origin/develop --head HEAD --format text
python3 tools/03_code_analysis/code_quality_score.py source/source_esolver/esolver_snap.cpp source/source_md/module_snap/snap_calculator.cpp source/source_md/module_snap/snap_descriptor_derivative.cpp source/source_md/module_snap/snap_model.cpp
git diff --cached --check

The Autotest command ran from /tmp/pr8235-regression/04_FF outside the sandbox with all five checked-in SNAP cases copied there. Build passed; 6/6 focused CTest tests passed; 20/20 numerical case comparisons passed. The four quality-scored changed files passed. The governance checker returned no blocking errors; its review warnings are addressed below. The three-atom case gives 54.909561 eV and Pxx -122.909021 kbar, consistent with the LAMMPS reference in its README; its force components and strain derivative are checked by the unit test. A direct UnitCell single-point run gave the same energy, force, and stress.

Governance notes

  • The PR-level GlobalV/GlobalC/PARAM dependency budget is net -2. The remaining GlobalV references select the established running log and gate rank-zero path confirmations.
  • New SNAP headers include types stored by value or used directly in their declarations; their include warnings require reviewer attention.
  • The final UnitCell refactor changes internal assembly only; the user-facing SNAP description is in docs/advanced/md.md.
  • No new C++ source file was added in the final refactor, so build lists do not change there.
  • Full MPI scaling and GPU validation were not run. The CPU regression covers periodic ghost-force return in one process. The local environment does not have pre-commit installed, so the governance checker was run directly.

@mohanchen mohanchen added MD & LAM MD and Larege Atomic Models Features Needed The features are indeed needed, and developers should have sophisticated knowledge Machine Learning & AI Issues related to AI labels Oct 29, 2026

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The SNAP MD path mixes ghost and owned force units, and accepted model settings can produce inconsistent or non-finite results. Please correct the force handoff and validate or handle those model settings.

Static source and analytic review at 47543b3 against e5ad530, with separate compatibility inspection of develop 5a7e6a6. No build, tests, numerical/MPI runs or CI diagnostics were executed; the author’s reported checks were not independently verified.

Non-blocking: Correct the PR INPUT notes: snap_param_file is newly added, and snap becomes an accepted esolver_type value. The C++ metadata registrations are already present.

Comment thread source/source_esolver/esolver_snap.cpp Resolved
Comment thread source/source_esolver/esolver_snap.cpp Resolved

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One radial edge case and three nonblocking items remain; the existing radial, chemical-order and argument-style threads contain the follow-ups. Source inspection confirms the ghost-force conversion, pair-cutoff checks, inner switch, common cell accumulator, environment reuse, logging and angular/strain test additions.

Static review at 8fbec3f; no builds, runtime tests or CI diagnostics were run. Author-reported test results were not independently reproduced.

Comment on lines +133 to +136
for (std::size_t index = 0; index < entries_.size(); ++index)
{
const Entry& entry = entries_[index];
if (entry.j1 == j1 && entry.m1 == m1 && entry.j2 == j2 && entry.m2 == m2 && entry.j == j && entry.m == m)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Non-blocking: coefficient() linearly searches entries_, and absent tuples traverse the whole vector. Both bispectrum contractions repeat these angular-index lookups across chemical channels and neighbor-coordinate derivatives. Pre-indexing admissible tuples or caching the contraction plan would avoid those repeated scans.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Features Needed The features are indeed needed, and developers should have sophisticated knowledge Machine Learning & AI Issues related to AI MD & LAM MD and Larege Atomic Models

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants