Skip to content

feat: add CPU SNAP potential support - #8235

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

19hello wants to merge 36 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_cg.cpp source/source_md/module_snap/snap_derivative.cpp source/source_md/module_snap/snap_descriptor.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 six 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.
  • Source files were renamed in the follow-up; CMake and Makefile object lists were updated. pyabacus has no SNAP entries to update.
  • 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.

Follow-up: model filenames and source naming (2026-11-02)

  • Kept the model fixtures under tests/SIMULATION_MODELS and named each pair by its CPU regression case number: snap_102, snap_103, snap_104, snap_105, and snap_106 (.coeff and .param). Each case INPUT points to its matching pair. Identical fixture contents are retained where two cases intentionally use the same potential, so the case-to-file mapping is explicit.
  • Restored source/source_md/module_snap/unittest and descriptive SNAP source, test, class, function, and variable names. The derivative implementation uses the shorter but descriptive snap_derivative.cpp. CMake and the legacy Makefile object list were updated; pyabacus has no SNAP-specific source entries.
  • Low-level SNAP code keeps typed C++ exceptions for library callers and unit tests. ESolver_SNAP::runner catches them and calls WARNING_QUIT, so runtime errors give a clear diagnostic on stdout and in warning.log and the running log.
  • Fresh verification after these naming changes: CMake configure and full build passed; 6/6 focused CTest tests passed; the five CPU SNAP cases passed all 20 numerical comparisons. The Autotest command above ran from /tmp/pr8235-regression/04_FF with tests/SIMULATION_MODELS copied to the parent directory.
  • Negative runtime check: a 2 Å pair with rmin0 2.1 exited 1 with SNAP evaluation failed: SNAP neighbor distance must be greater than rmin0. The message appeared in stdout, OUT.autotest/warning.log, and running_md.log.
  • The PR-level governance checker found no blocking errors. All six checked C++ files passed code_quality_score.py.

Follow-up: Clebsch–Gordan filenames (2026-11-02)

Renamed the implementation and header to snap_cg.cpp/snap_cg.h, the matching test source to test_snap_cg.cpp, and its CTest target to MODULE_MD_SNAP_cg. Updated the descriptor include, CMake source lists, and legacy Makefile object list. This is a naming-only change: full CMake build and all 4 SNAP unit tests passed. The staged governance check had no blocking errors; its header warning reflects the required complete CG table type in snap_descriptor.h, and no user documentation change is needed.

Follow-up: review comments and CI rerun (2026-11-02)

  • Skip zero-weight SNAP neighbors before the radial-domain guard in the analytic derivative, matching energy evaluation; retain the guard for contributing neighbors. Added ZeroWeightSkipsInvalidRadius to check isolated energy, zero forces, and nonzero-weight rejection.
  • Added ChemicalAngularChannelsFollowElementOrder with twojmax=2, a mixed chemical triplet, both neighbor forces versus finite differences, and equivalent energy/forces after swapping element order and permuting coefficient axes.
  • Bound weighted Dual values and both force unit conversions to named locals. Indexed precomputed Clebsch–Gordan tuples for repeated bispectrum lookups; the coefficient formula and numerical order are unchanged. No user-facing documentation change is required for these internal fixes.
  • Fresh final-head local verification: cmake --build /tmp/abacus-snap-check2 -j2 passed; ctest --test-dir /tmp/abacus-snap-check2 --output-on-failure -R 'MODULE_MD_SNAP_|MODULE_IO_(read_input_serial|read_item_serial)' passed 6/6; OMP_NUM_THREADS=1 bash ../integrate/Autotest.sh -a /tmp/abacus-snap-check2/abacus_pw_ser -n 1 -r '10[23456]_SNAP_' passed 20/20 numerical comparisons from /tmp/pr8235-regression/04_FF outside the sandbox. git diff origin/develop...HEAD --check and the PR-level governance checker passed without blocking errors. The new map and tuple header includes own a value member and are required. GitHub Integration Test and Unit Test job was explicitly rerun; the push also requests fresh CI on this head.

Follow-up: explicit SNAP channels and MPI errors (2026-11-02)

  • Removed the single-argument accumulate_u overload; callers now state the central element explicitly. Removed unused h_coefficient, coefficient_count, descriptor_size accessor, and the unconsumed derivative value array. These are internal cleanups; the user-facing SNAP model and INPUT syntax are unchanged.
  • Pass rank explicitly from the solver factory. ESolver_SNAP no longer reads GlobalV directly. Model paths are printed once each to rank-0 stdout and to the running log; these are two output destinations, not duplicate evaluations.
  • On a nonzero-rank fatal SNAP error, print the rank and reason to stderr before WARNING_QUIT, because the existing MPI setup disables nonzero-rank stdout and default per-rank logs. Added 04_FF_SNAP_rank_error, which places only the bad close pair on rank 1 and checks nonzero exit, rank-1 stderr, absence of the error on rank-0 stdout, and one stdout confirmation per model file.
  • The chemical-channel test now uses angle-sensitive mixed bispectrum index 2, checks energy changes when only the Si-C angle changes, and retains both force and element-permutation comparisons. Reused a named zero-coefficient vector.
  • Fresh checks: full serial CMake build passed; six focused CTests passed; five SNAP CPU regressions passed 20/20 numerical comparisons; full MPI abacus_pw_para build passed; LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu/openblas-pthread:$LD_LIBRARY_PATH ctest --test-dir /tmp/abacus-snap-build-develop-mpi --output-on-failure -R '^04_FF_SNAP_rank_error$' passed 1/1 outside the sandbox. The library path override is needed on this workstation because its system libblas.so.3 and toolchain OpenBLAS otherwise collide (undefined symbol: gotoblas). Python syntax check, diff whitespace check, and PR-level governance check passed without blocking errors. Governance notes: the factory is the existing top-level boundary where the global running log and rank are injected; the PR-level global dependency delta remains -2. No docs update is needed for the internal API cleanup; the stderr diagnostic improves error visibility.

Follow-up: model validation and descriptor reuse (2026-11-02)

  • Reject extra fields and trailing data in SNAP coefficient files; report coefficient-count mismatches through WARNING_QUIT with element, expected count, and actual count.
  • Reuse each central atom's accumulated U array, cache model-dependent B0 descriptors, and precompute quadratic force coefficients once per central atom.
  • Fresh verification on commit 74259aa10: cmake -S . -B build_parallel_md1_current -UENABLE_RAPIDJSON passed; cmake --build build_parallel_md1_current --target MODULE_MD_SNAP_model MODULE_MD_SNAP_calculator -j8, cmake --build build_parallel_md1_current --target MODULE_MD_SNAP_cg MODULE_MD_SNAP_descriptor -j8, cmake --build build_parallel_md1_current --target esolver -j8, and cmake --build build_parallel_md1_current --target abacus_basic_para -j8 passed; LD_LIBRARY_PATH=/tmp:$LD_LIBRARY_PATH ctest --test-dir build_parallel_md1_current --output-on-failure -R '^MODULE_MD_SNAP_' passed 4/4. The /tmp library path maps this host's broken system BLAS/LAPACK to the existing OpenBLAS library; without it, runtime linking fails with undefined symbol: gotoblas.
  • A fresh one-process run of case 105 from a /tmp copy passed with 54.909561 eV. A malformed 22-coefficient model (21 expected) exited 1 and wrote the mismatch to warning.log. git diff --check passed. The staged governance checker returned only a documentation-sync review warning; the user interface and documented behavior did not change.

Follow-up: rfac0 range (2026-11-02)

  • Validate 0 < rfac0 <= 1 in the SNAP model to keep the radial angle from crossing pi inside the pair cutoff. The zero-distance handling and std::domain_error path were left unchanged after discussion.
  • Document the range in docs/advanced/md.md; add direct boundary and parameter-file tests.
  • Fresh verification for commit 0a4bda26f: cmake --build build_parallel_md1_current --target MODULE_MD_SNAP_model MODULE_MD_SNAP_calculator -j8 passed; LD_LIBRARY_PATH=/tmp:$LD_LIBRARY_PATH ctest --test-dir build_parallel_md1_current --output-on-failure -R '^MODULE_MD_SNAP_' passed 4/4; cmake --build build_parallel_md1_current --target abacus_basic_para -j8 passed. One-process case 105 from a /tmp copy exited 0, and the same case with rfac0 1.2 exited 1 with SNAP rfac0 must satisfy 0 < rfac0 <= 1. in stdout. git diff --check passed; python3 tools/03_code_analysis/agent_governance_check.py --staged reported no findings. This host requires the temporary /tmp OpenBLAS mapping described above for runtime linking.

Follow-up: latest robot review and develop merge (2026-11-03)

  • Merged origin/develop in ae972f346. The only conflict was source/source_esolver/CMakeLists.txt; kept the upstream list format and retained esolver_snap.cpp. GitHub reports the PR as mergeable.
  • In 3a554aee0, added a two-element isolated chemical bzero regression with wselfallflag=0 and nonzero diagonal-channel coefficients; built the coefficient-count diagnostic in a named string; and restored the device summary to both stdout and the running log while collecting hardware information once. Replied to and resolved all three new robot review threads.
  • Fresh checks: cmake -S . -B build_parallel_md1_current; cmake --build build_parallel_md1_current --target abacus_basic_para -j8; LD_LIBRARY_PATH=/tmp:$LD_LIBRARY_PATH ctest --test-dir build_parallel_md1_current --output-on-failure -R '^MODULE_MD_SNAP_' (4/4 passed); LD_LIBRARY_PATH=/tmp:$LD_LIBRARY_PATH ctest --test-dir build_parallel_md1_current --output-on-failure -R '^04_FF_SNAP_rank_error$' (1/1 passed outside the sandbox). A one-process SNAP 105 run with OMP_NUM_THREADS=1 exited 0 at 54.909561 eV and printed the device summary once to stdout and once to running_md.log. The sandboxed MPI attempt could not launch because OpenMPI socket creation was denied; the outside-sandbox rerun passed. git diff --cached --check and the PR-level governance checker passed without blocking errors.

@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 thread tests/04_FF/105_SNAP_quad/STRU Resolved
Comment thread tests/04_FF/104_SNAP_inner/snap.coeff Resolved
@QuantumMisaka

Copy link
Copy Markdown
Collaborator

@19hello Only one question: why SNAP but not other CPU-running machine learning potential ?

@mohanchen

Copy link
Copy Markdown
Collaborator

@19hello Only one question: why SNAP but not other CPU-running machine learning potential ?

Someone needs this potential for research, more empirical/ML potentials will be added in future

@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 zero-weight guard, named numerical products and CG index address the previous requests in source. One MPI diagnostic gap remains below. Static review only; no build or runtime/MPI tests were run.

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.

Static incremental review at 0a4bda2 covered model validation, rank-visible errors, cached descriptors and analytic-force contractions. I did not run builds, tests or MPI calculations.

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.

4 participants