fix(pt-expt): detach tensors before numpy serialization

[njzjz]

Summary

• detach torch tensors that require gradients before exporting them through to_numpy_array
• document the PyTorch torch.asarray behavior change from pytorch/pytorch@a97dcf9
• add a regression test for serializing a trainable torch.nn.Parameter

Tests

• ruff check deepmd/dpmodel/common.py source/tests/consistent/test_array_api.py
• ruff format --check deepmd/dpmodel/common.py source/tests/consistent/test_array_api.py
• pytest source/tests/consistent/test_array_api.py::TestArrayConversion::test_torch_parameter_requires_grad -q
• pytest source/tests/consistent/test_array_api.py -q
• srun --gres=gpu:1 dp --pt-expt train input.json --skip-neighbor-stat
• srun --gres=gpu:1 dp --pt-expt train input.json

Summary by CodeRabbit

• Bug Fixes

  • Improved conversion of machine learning tensors to NumPy arrays when gradient tracking is enabled, preventing conversion failures while maintaining correct values.

• Tests

  • Added coverage for converting gradient-enabled PyTorch parameters, including checks for output values, dtype, and correct behavior before/after conversion.

[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: 413593f2-6551-451e-b5af-b550af839138

📥 Commits

Reviewing files that changed from the base of the PR and between 41e8af8 and 731326c.

📒 Files selected for processing (2)

• deepmd/dpmodel/common.py
• source/tests/consistent/test_array_api.py

✅ Files skipped from review due to trivial changes (1)

• deepmd/dpmodel/common.py

🚧 Files skipped from review as they are similar to previous changes (1)

• source/tests/consistent/test_array_api.py

---

📝 Walkthrough

Walkthrough

to_numpy_array in deepmd/dpmodel/common.py is updated to detect PyTorch tensors with requires_grad=True and call .detach() before np.asarray. A new TestArrayConversion test class verifies that conversion of a torch.nn.Parameter produces the correct NumPy values and dtype.

Changes

PyTorch grad-tensor detachment in to_numpy_array

Layer / File(s)	Summary
Grad-tensor detach fix and test deepmd/dpmodel/common.py, source/tests/consistent/test_array_api.py	to_numpy_array now calls .detach() on any torch tensor whose requires_grad is True before passing it to np.asarray; the existing DLPack fallback path is preserved. TestArrayConversion.test_torch_parameter_requires_grad creates a torch.nn.Parameter, converts it, and asserts the original tensor still has requires_grad=True while the result matches expected NumPy values and dtype.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 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%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: detaching PyTorch tensors before numpy serialization, which directly addresses the core issue in both modified files.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

source/tests/consistent/test_array_api.py (1)

41-43: ⚡ Quick win

Add an explicit dtype assertion in the regression test.

This test currently checks values and requires_grad, but it doesn’t strictly fail on dtype regression. Add a direct dtype check to match the stated regression scope.

Proposed test update

-        np.testing.assert_allclose(
-            to_numpy_array(param), np.array([1.0, 2.0], dtype=np.float64)
-        )
+        converted = to_numpy_array(param)
+        np.testing.assert_allclose(
+            converted, np.array([1.0, 2.0], dtype=np.float64)
+        )
+        self.assertEqual(converted.dtype, np.float64)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@source/tests/consistent/test_array_api.py` around lines 41 - 43, The
regression test in the assert_allclose block validates values but does not
explicitly check dtype to prevent regressions on data type changes. Add a direct
dtype assertion after the assert_allclose call that explicitly verifies the
dtype of the param matches the expected np.float64 dtype to ensure the dtype
regression scope is properly tested.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@source/tests/consistent/test_array_api.py`:
- Around line 41-43: The regression test in the assert_allclose block validates
values but does not explicitly check dtype to prevent regressions on data type
changes. Add a direct dtype assertion after the assert_allclose call that
explicitly verifies the dtype of the param matches the expected np.float64 dtype
to ensure the dtype regression scope is properly tested.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fbd5d63c-dac0-4f15-a2f4-4db12a02a9de

📥 Commits

Reviewing files that changed from the base of the PR and between 4b6506d and 41e8af8.

📒 Files selected for processing (2)

• deepmd/dpmodel/common.py
• source/tests/consistent/test_array_api.py

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.17%. Comparing base (4b6506d) to head (731326c).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5566   +/-   ##
=======================================
  Coverage   82.17%   82.17%           
=======================================
  Files         898      898           
  Lines      103576   103578    +2     
  Branches     4432     4436    +4     
=======================================
+ Hits        85117    85120    +3     
- Misses      17063    17066    +3     
+ Partials     1396     1392    -4     

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

[njzjz]

LGTM

— Opus 4.8