Skip to content

Comments

fix: omit virial in dp test summary if not available#4818

Merged
njzjz merged 1 commit intodeepmodeling:develfrom
caic99:patch-3
Jun 25, 2025
Merged

fix: omit virial in dp test summary if not available#4818
njzjz merged 1 commit intodeepmodeling:develfrom
caic99:patch-3

Conversation

@caic99
Copy link
Member

@caic99 caic99 commented Jun 24, 2025

Signed-off-by: Chun Cai <amoycaic@gmail.com>
Copilot AI review requested due to automatic review settings June 24, 2025 08:22
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 updates the test summary output to conditionally include virial statistics only when available in the avg dictionary.

  • Replaces if "...".keys() checks with direct key membership tests (if key in avg).
  • Adds a guard for printing virial metrics (mae_v, rmse_v, etc.) only if "rmse_v" exists.
  • Retains the existing conditional print for Hessian metrics using if "rmse_h" in avg.
Comments suppressed due to low confidence (2)

deepmd/entrypoints/test.py:608

  • Add unit tests for cases when virial data is present and when it's absent to ensure these conditional branches behave correctly and no unexpected KeyErrors occur.
    if "rmse_v" in avg:

deepmd/entrypoints/test.py:608

  • Guarding only on rmse_v may lead to a KeyError if other virial metrics like mae_v or mae_va exist independently. Consider checking a representative key that covers all virial data, or verify the presence of each metric before printing.
    if "rmse_v" in avg:

@codecov
Copy link

codecov bot commented Jun 24, 2025

Codecov Report

❌ Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.57%. Comparing base (04da98f) to head (2595c5e).
⚠️ Report is 81 commits behind head on devel.

Files with missing lines Patch % Lines
deepmd/entrypoints/test.py 42.85% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            devel    #4818   +/-   ##
=======================================
  Coverage   84.57%   84.57%           
=======================================
  Files         699      699           
  Lines       68035    68035           
  Branches     3540     3540           
=======================================
  Hits        57539    57539           
- Misses       9361     9364    +3     
+ Partials     1135     1132    -3     

☔ 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 added this pull request to the merge queue Jun 25, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 25, 2025
@njzjz njzjz added this pull request to the merge queue Jun 25, 2025
Merged via the queue into deepmodeling:devel with commit 205f924 Jun 25, 2025
59 checks passed
ChiahsinChu pushed a commit to ChiahsinChu/deepmd-kit that referenced this pull request Dec 17, 2025
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.

[BUG] dp test reports weighted avg of Virial errors even if the dataset does not contain virial info

2 participants