fix(pt): omit absent charge-spin inputs for legacy scripted models

[Zhang-pchao]

Summary

Do not pass charge_spin=None to scripted model methods that predate that optional argument. Preserve the keyword when a charge/spin tensor is supplied. Apply the same rule in inference and the model wrapper.

The regression loads the repository's legacy scripted fixture and compares native versus external (vesin) neighbor-list evaluation for energy, forces and virial.

Validation

Based on current master (7a9510d).

• pytest -q source/tests/pt/model/test_nlist_backend.py: 12 passed, 7 conditional GPU/backend skips. The new legacy-model regression executed and passed with vesin.
• All applicable repository pre-commit hooks passed, including pylint.

Tests used the proposed Python source with the existing compatible PyTorch 2.11 native operators. This is a Python schema-compatibility fix; a full native-backend rebuild was not required for that focused test.

Summary by CodeRabbit

• Bug Fixes
  • Improved compatibility with older scripted models when optional charge-spin data is omitted, while preserving explicitly supplied charge-spin values across inference and training.
  • Added a regression check for consistent evaluation results between native and external neighbor-list backends when using a legacy model.

CI follow-up (2026-10-07)

The exact-head Read the Docs build fails while installing doc/requirements.txt, before Sphinx runs: the selected PyTorch headers require C++20, while the current target baseline selects C++17. The same build-standard mismatch appears on all five related contribution heads (#6051 through #6055). Existing issue #6048 and PR #6049 already address this compatibility problem; this PR leaves that repair in its independent upstream review. The Read the Docs check remains failed until that dependency is resolved and the build is rerun. The exact-head pre-commit.ci check passes; GitHub Actions still require maintainer approval.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 75a3103b-c911-41fc-a11e-f5480295b715

📥 Commits

Reviewing files that changed from the base of the PR and between ae9f255 and 285932e.

📒 Files selected for processing (1)

• source/tests/pt/test_wrapper.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

DeepEval and ModelWrapper omit charge_spin when its value is None, while forwarding supplied values. Tests cover legacy scripted model compatibility and compare native and external neighbor-list evaluation.

Changes

Optional charge_spin compatibility

Layer / File(s)	Summary
Inference compatibility deepmd/pt/infer/deep_eval.py, source/tests/pt/model/test_nlist_backend.py	DeepEval omits charge_spin when it is None in both lower-model paths. A Vesin-gated test compares native and Vesin neighbor-list results for a legacy scripted model.
Wrapper compatibility deepmd/pt/train/wrapper.py, source/tests/pt/test_wrapper.py	ModelWrapper omits charge_spin from model inputs when it is None. Tests check calls to a scripted legacy-signature model and verify that supplied charge_spin values are forwarded in inference, skip-loss, and training modes.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 28593

No concrete merge-blocking risk introduced by this change was established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: omit absent charge-spin inputs to support legacy scripted models.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[njzjz-bot]

Agent: dot

Reviewed exact head ae9f255, including the full three-file patch, the native and external-neighbor-list dispatch, the modern model signatures, the energy-loss call path, and existing wrapper tests. No blocking correctness finding.

The compatibility change is consistent across the call sites: ModelWrapper omits only an absent charge_spin value, and both extended-coordinate and edge-schema lower calls do the same. A supplied tensor is retained. The modern EnergyModel/make_model and SeZM signatures default this argument to None, so omission preserves their existing missing-input/default behavior. The legacy fixture regression exercises both native wrapper dispatch and the Vesin lower path and compares energy, force and virial.

Nonblocking test suggestion: separate the native/wrapper compatibility assertion from the Vesin availability mark. Currently, without the optional Vesin dependency the whole new regression is skipped, including the native path. A small wrapper contract test using a legacy scripted signature, plus a modern toy model that asserts it receives the exact supplied charge_spin tensor, would protect both halves of this contract without the optional neighbor-list dependency. The existing wrapper toy accepts charge_spin but discards it, so it does not cover the forwarding guarantee.

Validation limit: this was a source review through the connector; I did not independently execute pytest or reproduce the PR's reported 12 passing tests. At review time the exact-head GitHub Actions runs reported action_required, pre-commit passed, and Read the Docs reported failure.

[Zhang-pchao]

Added a scripted toy model without a charge_spin argument, independent of Vesin, plus an identity check for an explicitly supplied tensor. Both cover inference, skip-loss and training. The wrapper and neighbor-list tests pass locally (19 passed, 7 conditional skips, 6 subtests passed). The existing Vesin parity test is retained.

[njzjz-bot]

Agent: dot

Reviewed delta ae9f255 → 285932e. No new blocking finding.

The added tests address my earlier suggestion: a TorchScript-compiled legacy model lacks charge_spin and runs through inference, skip-loss, and loss-mediated dispatch without Vesin. The modern toy asserts object identity of the supplied tensor in all three modes. Production code is unchanged.

Validation: full test-file/wrapper source review and Python syntax parsing. PyTorch is unavailable in this review environment, so these tests were not independently executed. Exact-head pre-commit passes, RTD fails, and seven Actions workflows remain action_required.