fix(pt): remap spins after null atom filtering

[njzjz]

Summary

• compact DeepSpinPT spin vectors with the same bkw_map used for coordinates and atom types
• add a TorchScript .pth neighbor-list regression with a NULL atom in the middle of the input
• use a strict representation-equivalence tolerance so both float and double instances detect the old mapping error

Root cause and fix

select_real_atoms_coord removes NULL-type atoms and returns compacted coordinates/types plus forward and backward maps. The DeepSpinPT neighbor-list path used the compacted coordinates and types, but reshaped a prefix of the original spin buffer to nall_real rows. That only preserves atom-to-spin association when all filtered NULL atoms happen to be at the end.

The fix rebuilds the spin buffer in bkw_map order before creating the tensor. Spin is a per-atom side channel, so it now follows exactly the same compacted ordering as coordinates and atom types, including local and ghost partitions.

Regression coverage

The existing tests covered ordinary neighbor-list inference, but did not place atype = -1 between real atoms. Therefore the incorrect prefix and the correct mapped spin buffers were indistinguishable in the tested layouts.

The new test evaluates the same six real atoms in two representations:

1. a compact input; and
2. a padded input with a distinctive NULL coordinate/spin inserted in the middle.

It compares energy, virial, real-atom force and magnetic force, and verifies that the NULL output slots are zero. The test explicitly checks the .pth backend so it exercises DeepSpinPT rather than PTExpt.

The file's historical float reference tolerance is 0.1. The old mapping error is smaller than that (maximum observed differences: energy 0.01493, force 0.02159, magnetic force 0.08961, virial 0.03228), so this equivalence regression uses 1e-10 for double and 1e-5 for float. With the old production line restored, both typed cases fail; with the fix, both pass.

Validation performed:

• built runUnitTests_cc, deepmd_backend_pt, and deepmd_op_pt with CPU PyTorch
• old implementation check: 0 passed, 2 failed as expected
• focused fixed regression: 2 passed
• broader NoPBC DeepSpinPT neighbor-list group: 6 passed
• clang-format 22.1.5 check
• ruff format --check .
• ruff check .
• git diff --check

Closes #5619.

Coding agent: Codex
Codex version: codex-cli 0.144.1
Model: gpt-5.6-sol
Reasoning effort: xhigh

Summary by CodeRabbit

• Bug Fixes

  • Corrected spin handling when NULL atoms are filtered from an input system.
  • Ensured spin data remains correctly mapped to real atoms, including when NULL atoms appear in the middle of the system.

• Tests

  • Added coverage verifying consistent energy, forces, and virial results for systems containing intermediate NULL atoms.
  • Confirmed NULL atoms receive zero forces.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f9fbec2d-304e-4a26-80b0-245afa955ae8

📥 Commits

Reviewing files that changed from the base of the PR and between 7d5ad38 and 088e103.

📒 Files selected for processing (2)

• source/api_cc/src/DeepSpinPT.cc
• source/api_cc/tests/test_deeppot_dpa_pt_spin.cc

---

📝 Walkthrough

Walkthrough

DeepSpinPT now remaps spin components alongside filtered real atoms using bkw_map. A typed CPU test verifies equivalent outputs when a NULL atom appears in the middle of a PyTorch neighbor list.

Changes

PyTorch spin remapping

Layer / File(s)	Summary
Remap spins during atom compaction source/api_cc/src/DeepSpinPT.cc	Spin components are copied into compact storage according to the real-atom mapping.
Validate NULL-atom neighbor-list compaction source/api_cc/tests/test_deeppot_dpa_pt_spin.cc	A typed CPU test compares compact and padded systems, including mapped forces, force magnitudes, virial values, and zero NULL-atom forces.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: bug, C++

Suggested reviewers: copilot, wanghan-iapcm

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: remapping spins after NULL-atom filtering.
Linked Issues check	✅ Passed	The fix and regression test match issue #5619 by remapping spins with bkw_map and covering a middle NULL atom case.
Out of Scope Changes check	✅ Passed	The PR stays focused on the spin remap bug and its regression test, with no evident unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 80.39216% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.57%. Comparing base (7d5ad38) to head (088e103).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
source/api_cc/tests/test_deeppot_dpa_pt_spin.cc	78.26%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5795      +/-   ##
==========================================
- Coverage   79.85%   79.57%   -0.28%     
==========================================
  Files        1022     1022              
  Lines      117351   117399      +48     
  Branches     4313     4316       +3     
==========================================
- Hits        93706    93422     -284     
- Misses      22101    22418     +317     
- Partials     1544     1559      +15     

☔ 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.

[wanghan-iapcm]

Correct fix. coord_wrapped = dcoord is compacted by select_real_atoms_coord with dcoord[ii] = coord[bkw_map[ii]], so rebuilding spin_wrapped[ii] = spin[bkw_map[ii]] gives spin exactly the same compacted ordering as coordinates and types; the old prefix of the original spin buffer was only correct when NULL atoms happened to be at the end. The regression is well designed: it uses the .pth backend explicitly (DeepSpinPT, not PTExpt), inserts a NULL atom with a distinctive spin in the middle so misalignment is observable, and -- importantly -- tightens the tolerance to 1e-10/1e-5, below the 0.015-0.09 error magnitude, since the file's default 0.1 tolerance would have masked it. It covers both float and double and checks that NULL output slots are zero. LGTM.

Dismissing to re-review through the /code-review skill per process.

[wanghan-iapcm]

Re-reviewed via the code-review skill. Correct and complete: coord_wrapped=dcoord is compacted so dcoord[ii]=coord[bkw_map[ii]], and rebuilding spin_wrapped[ii]=spin[bkw_map[ii]] gives spin the identical compacted ordering (same bkw_map gather/scatter idiom the output side already uses); the old prefix was only correct when NULLs were at the end. The incomplete-fix sweep is clean: DeepSpinPTExpt already uses bkw_map (correct since #5370) and DeepSpinTF folds spin into the coord array before filtering, so neither is affected; the no-nlist compute() overload correctly keeps the plain copy. The regression fails pre-fix (author's run: 0 passed, 2 failed) and, importantly, tightens the tolerance to 1e-10/1e-5 -- below the 0.01-0.09 error -- since the file default 0.1 would have masked it; both float and double, .pth backend, NULL-slot forces asserted zero. Bug dates to #4321. LGTM.