fix: change eV/A to eV/Å for dp test#4978
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the unit notation in logging output from "eV/A" to "eV/Å" (using the proper angstrom symbol) for the dp test command. The change ensures correct scientific notation for force, hessian, dipole, polarizability, and WFC measurements.
- Updates force units from "eV/A" to "eV/Å" in logging statements
- Updates hessian units from "eV/A^2" to "eV/Å^2" in logging statements
- Updates dipole, polarizability, and WFC units from "eV/A" to "eV/Å" in logging statements
for more information, see https://pre-commit.ci
📝 WalkthroughWalkthroughReplaced occurrences of "eV/A" and "eV/A^2" with "eV/Å" and "eV/Å^2" in log/output strings within deepmd/entrypoints/test.py. No logic, data structure, or control-flow changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
deepmd/entrypoints/test.py (1)
1072-1074: Fix missing f-strings in WFC logs (values not interpolated).Both lines print the braces literally today. Make them f-strings.
- log.info("# number of test data : {numb_test:d} ") - log.info("WFC RMSE : {rmse_f:e} eV/Å") + log.info(f"# number of test data : {numb_test:d} ") + log.info(f"WFC RMSE : {rmse_f:e} eV/Å")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
deepmd/entrypoints/test.py(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Run ruff check and ruff format on all Python code before committing
Ensure Python code is formatted with ruff’s formatter
Files:
deepmd/entrypoints/test.py
🔇 Additional comments (7)
deepmd/entrypoints/test.py (7)
514-521: Units switch for Force logs looks good (Å).A→Å change is correct. Please also run ruff format/check before merge.
524-526: Force atom logs updated to Å — OK.
546-547: Hessian units updated to eV/Å^2 — OK.
666-671: System‑avg Force logs: A→Å change looks correct.
672-675: System‑avg Force atom logs: A→Å change looks correct.
682-683: System‑avg Hessian logs: A→Å^2 change looks correct.
1100-1100: System‑avg WFC log: A→Å change looks correct.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #4978 +/- ##
==========================================
- Coverage 84.20% 84.20% -0.01%
==========================================
Files 705 705
Lines 69229 69229
Branches 3576 3575 -1
==========================================
- Hits 58293 58292 -1
Misses 9797 9797
- Partials 1139 1140 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
change the unit eV/A to eV/Å for dp test <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Bug Fixes - Standardized unit display from eV/A and eV/A^2 to eV/Å and eV/Å^2 across all distance-related logs. Affects force metrics (MAE/RMSE, weighted, per-atom), Hessian, WFC, polarizability, dipole, and corresponding system-average summaries. Improves readability, aligns with scientific notation, and removes ambiguity in reported units. No changes to calculations or results—only log message text was updated. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
change the unit eV/A to eV/Å for dp test
Summary by CodeRabbit