-
Notifications
You must be signed in to change notification settings - Fork 585
feat: capitalise some info display #5145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughTitle-case and capitalization standardization of user-facing strings across logs and summaries: backend-info keys, training log prefix, data-system header, and summary/build labels. No changes to control flow, public signatures, or underlying values. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ 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). (20)
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
deepmd/utils/data_system.py (1)
716-716: Consider the behavior ofcapitalize()on system names.The
capitalize()method lowercases all characters except the first, which may not preserve the intended casing of system names. For example, "MySystem" becomes "Mysystem", and mixed-case acronyms or proper names would be affected.Consider using
name.title()for word-based capitalization or simplynameif the original casing should be preserved.Alternative approaches
If word-based capitalization is desired:
- f"---Summary of DataSystem: {name.capitalize():13s}-----------------------------------------------" + f"---Summary of DataSystem: {name.title():13s}-----------------------------------------------"Or preserve original casing:
- f"---Summary of DataSystem: {name.capitalize():13s}-----------------------------------------------" + f"---Summary of DataSystem: {name:13s}-----------------------------------------------"
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
deepmd/loggers/training.pydeepmd/pd/entrypoints/main.pydeepmd/pt/entrypoints/main.pydeepmd/tf/train/run_options.pydeepmd/utils/data_system.pydeepmd/utils/summary.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-12T13:40:14.334Z
Learnt from: CR
Repo: deepmodeling/deepmd-kit PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T13:40:14.334Z
Learning: Verify PyTorch training output shows decreasing loss with 'batch X: trn: rmse' messages
Applied to files:
deepmd/loggers/training.py
📚 Learning: 2025-12-12T13:40:14.334Z
Learnt from: CR
Repo: deepmodeling/deepmd-kit PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T13:40:14.334Z
Learning: Verify TensorFlow training output shows decreasing loss with 'batch X: trn: rmse' messages
Applied to files:
deepmd/loggers/training.py
🧬 Code graph analysis (1)
deepmd/utils/data_system.py (1)
deepmd/utils/path.py (3)
name(143-144)name(264-266)name(466-468)
⏰ 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). (41)
- GitHub Check: CodeQL analysis (python)
- GitHub Check: Test Python (10, 3.10)
- GitHub Check: Test Python (10, 3.13)
- GitHub Check: Test Python (9, 3.13)
- GitHub Check: Test Python (8, 3.13)
- GitHub Check: Test Python (11, 3.10)
- GitHub Check: Test Python (7, 3.13)
- GitHub Check: Test Python (5, 3.10)
- GitHub Check: Test Python (11, 3.13)
- GitHub Check: Test Python (12, 3.13)
- GitHub Check: Test Python (9, 3.10)
- GitHub Check: Test Python (12, 3.10)
- GitHub Check: Test Python (8, 3.10)
- GitHub Check: Test Python (6, 3.13)
- GitHub Check: Test Python (5, 3.13)
- GitHub Check: Test Python (1, 3.10)
- GitHub Check: Test Python (4, 3.13)
- GitHub Check: Test Python (2, 3.13)
- GitHub Check: Test Python (7, 3.10)
- GitHub Check: Test Python (4, 3.10)
- GitHub Check: Test Python (3, 3.13)
- GitHub Check: Test Python (6, 3.10)
- GitHub Check: Test Python (2, 3.10)
- GitHub Check: Test Python (1, 3.13)
- GitHub Check: Test Python (3, 3.10)
- GitHub Check: Build wheels for cp310-manylinux_aarch64
- GitHub Check: Build wheels for cp311-macosx_arm64
- GitHub Check: Build wheels for cp311-macosx_x86_64
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Build wheels for cp311-win_amd64
- GitHub Check: Test C++ (false, true, true, false)
- GitHub Check: Test C++ (true, true, true, false)
- GitHub Check: Test C++ (false, false, false, true)
- GitHub Check: Test C++ (true, false, false, true)
- GitHub Check: Build C library (2.18, libdeepmd_c.tar.gz)
- GitHub Check: Build C++ (cuda120, cuda)
- GitHub Check: Build C++ (clang, clang)
- GitHub Check: Build C++ (rocm, rocm)
- GitHub Check: Build C++ (cpu, cpu)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (c-cpp)
🔇 Additional comments (9)
deepmd/utils/data_system.py (1)
718-718: LGTM!The capitalization change to "Found X System(s):" is consistent with the PR's objective to standardize display formatting.
deepmd/loggers/training.py (2)
15-15: LGTM!The capitalization of "Batch" improves consistency with the PR's formatting objectives.
52-52: LGTM!The capitalization of "Batch" is consistent with the change at Line 15 and aligns with the PR's display standardization goals.
deepmd/tf/train/run_options.py (1)
70-73: LGTM!The backend info dictionary key renames follow a consistent title case convention, aligning with the PR's objective to standardize display formatting. The underlying values remain unchanged.
deepmd/pd/entrypoints/main.py (1)
222-223: LGTM!The backend info dictionary key renames standardize capitalization ("PD Ver") and improve clarity ("Custom OP Enabled" better describes the state). These changes are consistent with similar updates in other backend modules.
deepmd/pt/entrypoints/main.py (2)
242-244: LGTM!The backend info dictionary key renames apply consistent title case formatting, aligning with the standardization effort across all backend modules in this PR.
250-251: LGTM!The key renames ("PT Ver" and "Custom OP Enabled") are consistent with similar changes in the PD and TF backend modules, creating a unified display format across all backends.
deepmd/utils/summary.py (2)
67-68: Consistent capitalization improves display readability.The key renamings in the
__call__method maintain consistency with the BUILD dictionary updates:
- Multi-node keys use Title Case ("World Size", "Node List")
- Runtime environment keys use Title Case ("Running On", "Computing Device")
- GPU count key reordered for better readability ("Visible GPU Count")
- Thread keys use ALL_CAPS ("NUM_INTRA_THREADS", "NUM_INTER_THREADS"), consistent with environment variable naming conventions
The
.upper()transformation on the computing device value enhances visual emphasis in the output.Also applies to: 73-74, 84-84, 89-90
49-57: No breaking changes are introduced by the BUILD dictionary key modifications. While the keys have been renamed (e.g., "installed to" → "Installed To", "use float prec" → "Float Precision"), no code in the codebase accesses these keys directly. The BUILD dictionary is used solely in the__call__method for generating display output; it is not exposed as a public API that external code depends on. The value transformations (.capitalize()and.upper()) are also display-only changes that do not affect functionality.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5145 +/- ##
==========================================
- Coverage 81.94% 81.94% -0.01%
==========================================
Files 713 713
Lines 73009 73009
Branches 3617 3616 -1
==========================================
- Hits 59826 59824 -2
Misses 12021 12021
- Partials 1162 1164 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this 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 standardizes capitalization and formatting of display labels across the DeepMD-kit system to improve visual consistency. The changes focus on making information displays tidier by applying consistent capitalization conventions to build information, backend details, data system summaries, and training batch messages.
Changes:
- Standardized label capitalization in build information and runtime configuration displays to Title Case
- Updated backend information labels across TensorFlow, PyTorch, and Paddle backends for consistency
- Capitalized logging messages in data system summaries and training batch outputs
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| deepmd/utils/summary.py | Updated BUILD dictionary keys and runtime info labels to Title Case; applied uppercase transformation to variant and device values |
| deepmd/utils/data_system.py | Capitalized data system summary log messages |
| deepmd/tf/train/run_options.py | Standardized TensorFlow backend information labels to Title Case |
| deepmd/pt/entrypoints/main.py | Standardized PyTorch backend information labels to Title Case |
| deepmd/pd/entrypoints/main.py | Standardized Paddle backend information labels to Title Case |
| deepmd/loggers/training.py | Capitalized "Batch" in training progress messages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
(cherry picked from commit 7b0cb9f)
Co-authored-by: Copilot <[email protected]> Signed-off-by: OutisLi <[email protected]>
To make display tidier.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.