Skip to content

fix(dpmodel): fix rmse_* in more_loss#5105

Merged
wanghan-iapcm merged 1 commit intodeepmodeling:develfrom
njzjz:fix-dpmodel-loss-rmse
Dec 16, 2025
Merged

fix(dpmodel): fix rmse_* in more_loss#5105
wanghan-iapcm merged 1 commit intodeepmodeling:develfrom
njzjz:fix-dpmodel-loss-rmse

Conversation

@njzjz
Copy link
Member

@njzjz njzjz commented Dec 15, 2025

The previous value was MSE.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed RMSE metric calculations to correctly display square-root values for energy, force, virial, and atom-related loss measurements, ensuring accurate performance metrics reporting.

✏️ Tip: You can customize this high-level summary in your review settings.

The previous value was MSE.
Copilot AI review requested due to automatic review settings December 15, 2025 15:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where RMSE (Root Mean Squared Error) metrics in the more_loss dictionary were incorrectly storing MSE (Mean Squared Error) values. The fix adds xp.sqrt() to convert the L2 loss values to proper RMSE values.

  • Wraps all l2_*_loss variables with xp.sqrt() when assigning to rmse_* keys in more_loss
  • Ensures consistency with the existing more_loss["rmse"] calculation at line 268
  • Applies the fix uniformly across all six RMSE metrics: energy, force, virial, atom energy, prefactor force, and generalized force

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 15, 2025

📝 Walkthrough

Walkthrough

Modified deepmd/dpmodel/loss/ener.py to display RMSE metrics as the square root of L2 loss values instead of raw L2 values across energy, force, virial, and related loss components.

Changes

Cohort / File(s) Summary
RMSE Metric Calculation Correction
deepmd/dpmodel/loss/ener.py
Applied square-root transformation to L2 loss values for RMSE display across multiple components: energy, force, virial, atom energy, atom preference force, and generalized force. All metrics now properly display root-mean-square error values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify mathematical correctness: ensure sqrt is applied consistently to all relevant L2 loss metrics
  • Confirm no unintended side effects on loss calculation logic or backward compatibility
  • Check that conditional existence checks via display_if_exist still function as expected with the wrapped values

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly matches the PR's main objective: fixing rmse_* metrics in more_loss by replacing MSE values with their square-root counterparts.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b9c82d7 and 0d6fe16.

📒 Files selected for processing (1)
  • deepmd/dpmodel/loss/ener.py (6 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: 1azyking
Repo: deepmodeling/deepmd-kit PR: 4169
File: deepmd/pt/loss/ener_hess.py:341-348
Timestamp: 2024-10-05T03:11:02.922Z
Learning: In `deepmd/pt/loss/ener_hess.py`, the `label` uses the key `"atom_ener"` intentionally to maintain consistency with the forked version.
Learnt from: 1azyking
Repo: deepmodeling/deepmd-kit PR: 4169
File: deepmd/pt/loss/ener_hess.py:341-348
Timestamp: 2024-10-08T15:32:11.479Z
Learning: In `deepmd/pt/loss/ener_hess.py`, the `label` uses the key `"atom_ener"` intentionally to maintain consistency with the forked version.
Learnt from: 1azyking
Repo: deepmodeling/deepmd-kit PR: 4169
File: deepmd/utils/argcheck.py:1982-2117
Timestamp: 2024-10-05T03:06:02.372Z
Learning: The `loss_ener_hess` and `loss_ener` functions should remain separate to avoid confusion, despite code duplication.
📚 Learning: 2024-10-08T15:32:11.479Z
Learnt from: 1azyking
Repo: deepmodeling/deepmd-kit PR: 4169
File: deepmd/pt/loss/ener_hess.py:341-348
Timestamp: 2024-10-08T15:32:11.479Z
Learning: In `deepmd/pt/loss/ener_hess.py`, the `label` uses the key `"atom_ener"` intentionally to maintain consistency with the forked version.

Applied to files:

  • deepmd/dpmodel/loss/ener.py
📚 Learning: 2024-10-05T03:06:02.372Z
Learnt from: 1azyking
Repo: deepmodeling/deepmd-kit PR: 4169
File: deepmd/utils/argcheck.py:1982-2117
Timestamp: 2024-10-05T03:06:02.372Z
Learning: The `loss_ener_hess` and `loss_ener` functions should remain separate to avoid confusion, despite code duplication.

Applied to files:

  • deepmd/dpmodel/loss/ener.py
🧬 Code graph analysis (1)
deepmd/dpmodel/loss/ener.py (3)
deepmd/dpmodel/loss/loss.py (1)
  • display_if_exist (40-58)
deepmd/pt/loss/loss.py (1)
  • display_if_exist (44-54)
deepmd/tf/loss/loss.py (1)
  • display_if_exist (78-92)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
  • GitHub Check: CodeQL analysis (python)
  • GitHub Check: Test C++ (false, false, false, true)
  • GitHub Check: Test C++ (false, true, true, false)
  • GitHub Check: Test C++ (true, false, false, true)
  • GitHub Check: Test C++ (true, true, true, false)
  • GitHub Check: Test Python (1, 3.10)
  • GitHub Check: Test Python (2, 3.10)
  • GitHub Check: Test Python (5, 3.10)
  • GitHub Check: Test Python (5, 3.13)
  • GitHub Check: Test Python (4, 3.13)
  • GitHub Check: Test Python (6, 3.10)
  • GitHub Check: Test Python (6, 3.13)
  • GitHub Check: Test Python (3, 3.10)
  • GitHub Check: Test Python (1, 3.13)
  • GitHub Check: Test Python (3, 3.13)
  • GitHub Check: Test Python (4, 3.10)
  • GitHub Check: Test Python (2, 3.13)
  • GitHub Check: Build C++ (clang, clang)
  • GitHub Check: Build C++ (rocm, rocm)
  • GitHub Check: Build C++ (cuda120, cuda)
  • GitHub Check: Build C library (2.18, libdeepmd_c.tar.gz)
  • GitHub Check: Build C++ (cpu, cpu)
  • GitHub Check: Build wheels for cp310-manylinux_aarch64
  • GitHub Check: Build wheels for cp311-macosx_arm64
  • GitHub Check: Build wheels for cp311-win_amd64
  • GitHub Check: Build wheels for cp311-macosx_x86_64
  • GitHub Check: Build wheels for cp311-manylinux_x86_64
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (c-cpp)
🔇 Additional comments (1)
deepmd/dpmodel/loss/ener.py (1)

182-184: LGTM! RMSE calculations are now mathematically correct.

All six RMSE metrics (rmse_e, rmse_f, rmse_v, rmse_ae, rmse_pf, rmse_gf) now correctly compute the square root of their respective L2 losses, properly converting MSE to RMSE values. Note that the PyTorch backend (deepmd/pt/loss/ener.py) already implements this correctly with .sqrt() calls, and the TensorFlow backend uses a different post-processing architecture with np.sqrt().


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

@codecov
Copy link

codecov bot commented Dec 15, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.14%. Comparing base (b9c82d7) to head (0d6fe16).

Files with missing lines Patch % Lines
deepmd/dpmodel/loss/ener.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #5105      +/-   ##
==========================================
- Coverage   82.14%   82.14%   -0.01%     
==========================================
  Files         709      709              
  Lines       72453    72454       +1     
  Branches     3616     3615       -1     
==========================================
  Hits        59515    59515              
- Misses      11775    11776       +1     
  Partials     1163     1163              

☔ View full report in Codecov by Sentry.
📢 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 requested a review from wanghan-iapcm December 16, 2025 06:12
@wanghan-iapcm wanghan-iapcm added this pull request to the merge queue Dec 16, 2025
Merged via the queue into deepmodeling:devel with commit 89a3180 Dec 16, 2025
64 checks passed
ChiahsinChu pushed a commit to ChiahsinChu/deepmd-kit that referenced this pull request Dec 17, 2025
The previous value was MSE.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Fixed RMSE metric calculations to correctly display square-root values
for energy, force, virial, and atom-related loss measurements, ensuring
accurate performance metrics reporting.

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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.

3 participants