fix(dpmodel): fix energy loss #4765
Merged
+64
−65
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. dpmodel has different model output keys; 2. in the current code, energy, force, virial, etc are necessary keys. Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR updates the dpmodel loss components to use new output key names and ensures that energy, force, virial, and atom energies are correctly mapped.
- Updated the test suite to use a new prediction mapping dictionary (predict_dpmodel_style)
- Re-mapped keys in loss computation in ener.py to match the new dpmodel output
- Removed conditional checks in label_requirement to enforce mandatory keys
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| source/tests/consistent/loss/test_ener.py | Introduces a new prediction mapping (predict_dpmodel_style) to test dpmodel outputs using new key names |
| deepmd/dpmodel/loss/ener.py | Adjusts the key mappings in the loss function and unconditionally requires related keys in label_requirement |
Comments suppressed due to low confidence (1)
deepmd/dpmodel/loss/ener.py:268
- Since the conditional check has been removed, update the function documentation in label_requirement to reflect that energy, force, virial, and atom energy keys are now mandatory.
- if self.has_e:
Contributor
📝 Walkthrough## Walkthrough
The changes update the keys used to access model outputs and label requirements in the `EnergyLoss` class and its corresponding tests. The new keys reflect a different expected output structure from the model, and the test code is updated to use these keys when preparing prediction dictionaries for evaluation.
## Changes
| File(s) | Change Summary |
|-------------------------------------------|-------------------------------------------------------------------------------------------------------------------|
| deepmd/dpmodel/loss/ener.py | Updated `EnergyLoss` to use new model output keys (`energy_redu`, `energy_derv_r`, `energy_derv_c_redu`, `energy`) in `call` and `label_requirement`. Renamed loss reporting keys and added total RMSE key. |
| source/tests/consistent/loss/test_ener.py | Updated test class to use a new prediction dictionary (`predict_dpmodel_style`) with the new key scheme in relevant evaluation methods. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Test as TestEner
participant Model as EnergyLoss
participant ModelOutput as Model Output Dict
Test->>Model: call(model_dict with new keys)
Model->>ModelOutput: Access energy_redu, energy_derv_r, energy_derv_c_redu, energy
ModelOutput-->>Model: Return values
Model-->>Test: Return loss calculationPossibly related PRs
Suggested reviewers
Length of output: 41408 Verify weighted RMSE vs. true RMSE more_loss["rmse"] = xp.sqrt(loss)computes √(∑ᵢ wᵢ·MSEᵢ) rather than a conventional RMSE (√(∑ errors² / N)). This matches the Paddle/PyTorch backends but only yields a “weighted RMSE” when your component weights ( Please confirm whether you intend to:
If a pure RMSE is desired, adjust the formula accordingly. Locations to review:
Details❓ Verification inconclusiveReview the impact of removing conditional checks in label_requirement. The label requirements are now added unconditionally, removing the previous conditional logic based on This change may impact performance and data loading behavior. Please verify:
🏁 Script executed: #!/bin/bash
# Description: Compare label_requirement implementations across different loss modules
echo "=== Checking other loss implementations for conditional patterns ==="
rg -A 20 -B 5 'def label_requirement' --type py
rg -A 10 'has_e.*has_f.*has_v' --type py
echo "=== Checking if dpmodel requires different handling ==="
rg -A 10 -B 5 'dpmodel.*label.*requirement' --type pyLength of output: 25223 Review deepmd/dpmodel/loss/ener.py – Unconditional label_requirement additions The Please verify:
• File: deepmd/dpmodel/loss/ener.py – ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #4765 +/- ##
==========================================
+ Coverage 84.79% 84.80% +0.01%
==========================================
Files 698 698
Lines 67734 67742 +8
Branches 3540 3540
==========================================
+ Hits 57432 57446 +14
+ Misses 9171 9166 -5
+ Partials 1131 1130 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
wanghan-iapcm
approved these changes
May 26, 2025
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
for more information, see https://pre-commit.ci
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit