Skip to content

fix(model): register magnetic cell derivatives correctly#5836

Open
njzjz-bot wants to merge 1 commit into
deepmodeling:masterfrom
njzjz-bot:fix/magnetic-cell-derivative-output-def-5632
Open

fix(model): register magnetic cell derivatives correctly#5836
njzjz-bot wants to merge 1 commit into
deepmodeling:masterfrom
njzjz-bot:fix/magnetic-cell-derivative-output-def-5632

Conversation

@njzjz-bot

@njzjz-bot njzjz-bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • register magnetic cell derivatives in the cell-derivative bucket instead of the coordinate bucket
  • generate the corresponding reduced magnetic virial definition
  • add the optional DeepEval backend name mapping so spin evaluation request construction remains valid
  • extend output-definition tests for bucket membership, shape, category, reduction, and atomic metadata

Why existing tests missed this

The existing comprehensive output-definition test only compared the flattened ModelOutputDef key set. The magnetic cell derivative appeared in that flattened set even while stored in the wrong internal bucket, so the test passed. It did not inspect keys_derv_c versus keys_derv_r and did not expect the reduced magnetic virial definition. The new regression asserts both internal bucket membership and the derived reduced definition.

Validation

  • ruff format .
  • ruff check .
  • complete output-definition test file plus TensorFlow C++ core regression: 17 passed
  • verified the new reduced output has a DeepEval backend mapping; unsupported backend values keep the existing optional-output NaN/removal behavior

An additional pt_expt spin-test collection attempt hit an environment Triton plugin import segmentation fault before test collection; the focused pure-metadata and core tests are unaffected.

Closes #5632

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

Summary by CodeRabbit

  • Bug Fixes
    • Corrected magnetic cell-derivative outputs so they are registered and reduced through the appropriate cell-based pathway.
    • Added support for magnetic cell-derivative output mapping to backend virial results.
    • Ensured magnetic derivative outputs have the correct reduced shape and classification.

Place magnetic cell derivatives in the cell bucket so reduced definitions are generated. Add the optional DeepEval virial mapping and bucket/category regression coverage.

Coding-Agent: Codex
Codex-Version: codex-cli 0.144.4
Model: gpt-5.6-sol
Reasoning-Effort: xhigh
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 4ec05890-ead4-4eac-aed6-0652f0ac83da

📥 Commits

Reviewing files that changed from the base of the PR and between 6c3b985 and 903c96b.

📒 Files selected for processing (3)
  • deepmd/dpmodel/output_def.py
  • deepmd/infer/deep_eval.py
  • source/tests/common/dpmodel/test_output_def.py

📝 Walkthrough

Walkthrough

Magnetic cell derivatives are moved to the cell-derivative bucket, mapped to the backend’s virial_mag output, and covered by expanded output-definition tests.

Changes

Magnetic cell derivative outputs

Layer / File(s) Summary
Cell-derivative registration
deepmd/dpmodel/output_def.py
Magnetic derivatives for cell-differentiable variables are stored in def_derv_c instead of def_derv_r.
Backend mapping and regression coverage
deepmd/infer/deep_eval.py, source/tests/common/dpmodel/test_output_def.py
The reduced magnetic cell derivative maps to virial_mag; tests verify its presence, shape, atomicity, category flags, reduction behavior, and invalid repeated reduction.

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

Suggested labels: Python

Suggested reviewers: 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 fix: magnetic cell derivatives are now registered correctly.
Linked Issues check ✅ Passed The changes fix the bucket placement bug, enable the reduced magnetic cell derivative, and add regression coverage as requested in #5632.
Out of Scope Changes check ✅ Passed The additional backend mapping and test updates are directly related to the magnetic derivative fix and do not appear out of scope.
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

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

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.86%. Comparing base (6c3b985) to head (903c96b).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5836      +/-   ##
==========================================
- Coverage   78.58%   76.86%   -1.73%     
==========================================
  Files        1050     1050              
  Lines      120637   120634       -3     
  Branches     4356     4358       +2     
==========================================
- Hits        94801    92721    -2080     
- Misses      24278    26350    +2072     
- Partials     1558     1563       +5     

☔ 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

njzjz commented Jul 18, 2026

Copy link
Copy Markdown
Member

Possible reviewers based on changed lines, exact file history, and exact-file review history:

  • @wanghan-iapcm — 13 commits on changed files; 37 reviews on exact changed files (deepmd/dpmodel/output_def.py, deepmd/infer/deep_eval.py, source/tests/common/dpmodel/test_output_def.py).
  • @iProzd — 12 commits on changed files (deepmd/dpmodel/output_def.py, deepmd/infer/deep_eval.py, source/tests/common/dpmodel/test_output_def.py).

No review request was made automatically.

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

@njzjz
njzjz requested review from iProzd and wanghan-iapcm and removed request for iProzd and wanghan-iapcm July 18, 2026 07:15
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.

[Code scan] Register magnetic cell derivatives in the cell-derivative bucket

2 participants