Skip to content

fix(lammps): register native-spin host pair style in plugin - #6012

Draft
LOGO127 wants to merge 2 commits into
deepmodeling:masterfrom
LOGO127:fix/register-native-spin-host
Draft

LOGO127 wants to merge 2 commits into
deepmodeling:masterfrom
LOGO127:fix/register-native-spin-host

Conversation

@LOGO127

@LOGO127 LOGO127 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Register the existing PairDPA4Spin host implementation as dpa4spin in the runtime plugin. Currently, a non-Kokkos plugin loads successfully but pair_style dpa4spin ... is unrecognized. The header's PairStyle declaration serves built-in packages; runtime plugins register their factories explicitly. The plugin already registers the Kokkos variants, but omits this host factory.

This adds the host factory and registration alongside the existing pair styles, plus an artifact-independent LAMMPS Python test asserting that the host style is available after loading the plugin. No model, force conversion, Kokkos alias or unit-policy changes are included.

Validation

Built against DeePMD master 28b7d068801716765ab8119257f814596e49a10c and LAMMPS stable_22Jul2025_update6 (9c5ab448c78a14fd534619622162ba418d6a1fb1), using a CPU serial shared LAMMPS build with PLUGIN and SPIN. The separate DeePMD build uses ALLOW_NO_BACKEND=ON, BUILD_CPP_IF=ON, BUILD_PY_IF=OFF, DP_USING_C_API=OFF, and LAMMPS_SOURCE_ROOT pointing to that checkout.

With the matching LAMMPS Python package on PYTHONPATH, its shared library on LD_LIBRARY_PATH, and DEEPMD_TEST_PLUGIN pointing to the compiled plugin:

python -m pytest source/lmp/tests/test_lammps_dpa4spin_registration.py -q

The exact new test fails on pristine upstream production sources because has_style("pair", "dpa4spin") is false. It passes with the candidate, including after restoring and rebuilding the candidate following the baseline comparison. Ruff 0.16.0, isort 9.0.0b1 checks and clang-format 22.1.8 dry-run pass for the changed files.

No model inference, GPU, MPI, Kokkos execution or full backend-suite coverage is claimed. This is intentionally separate from the native-spin unit-policy issue #5993 and does not close it.

AI assistance was used for diagnosis, implementation and validation. Opening as a draft for scope feedback; broader backend validation remains outstanding.

@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.04%. Comparing base (28b7d06) to head (ab5b5a4).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6012      +/-   ##
==========================================
- Coverage   77.25%   77.04%   -0.21%     
==========================================
  Files        1153     1154       +1     
  Lines      138930   139217     +287     
  Branches     5056     5056              
==========================================
- Hits       107328   107262      -66     
- Misses      29717    30069     +352     
- Partials     1885     1886       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@LOGO127

LOGO127 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Hi! I checked the completed CI for head 0198c0f. The cp311 manylinux x86_64 wheel job fails while Docker is creating/pulling the pinned quay.io/pypa/manylinux_2_28_x86_64 image, before wheel compilation:

Error response from daemon: received unexpected HTTP status: 502 Bad Gateway

The wheel aggregate is also red because of that failed prerequisite. The C++ build/test, C-library build, and Python test aggregate checks have completed successfully. The CUDA test jobs themselves were skipped, so I am not counting the green CUDA aggregate as executed GPU coverage.

Could a maintainer please rerun the failed jobs in run 34212525250 when convenient? I do not have repository write permissions. I have left the source and CI configuration unchanged; the image-acquisition failure does not establish a source regression, nor does it establish that the wheel will pass after retry. Thank you! This log investigation and follow-up were prepared with AI assistance.

@njzjz-bot njzjz-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the complete two-file draft. The production registration itself is appropriately scoped and matches the existing plugin factory pattern, but the regression test can currently false-pass without exercising the plugin registration. I left one inline comment on that test. The separate native-spin unit-policy bug in #5993 is outside this PR's scope and is not a blocker here.

Most exact-head CI is green; the PyPI workflow failure is an external manylinux image 502 reported before wheel compilation, so I am not treating it as source evidence. This remains a draft, so no APPROVE/REQUEST_CHANGES disposition is submitted.


Agent: ChatGPT
Model: GPT-5.6 Sol
GitHub account: njzjz-bot
Reviewed head: 0198c0f
Trigger: scheduled all-PR monitoring

Comment thread source/lmp/tests/test_lammps_dpa4spin_registration.py Outdated

@njzjz-bot njzjz-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed the full two-file change at this new head. The previous regression-test gap is fixed: the test now requires DEEPMD_TEST_PLUGIN, verifies dpa4spin is absent from the host before loading the plugin, loads the requested runtime plugin, and then verifies the style appears. That makes the behavior under test specifically the new runtime-plugin registration rather than a built-in style.

I do not see a new high-confidence correctness blocker in this head. I am leaving this as COMMENT because the PR is still draft and the exact-head Test Python, Test C++, CodeQL, and PyPI workflows are still in progress; Build C++, Build C library, and Test CUDA have passed.


Agent: ChatGPT
Model: GPT-5.6 Sol
GitHub account: njzjz-bot
Reviewed head: ab5b5a4
Trigger: scheduled all-PR monitoring

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants