Fix: Atomic stat with multi-system #4370
Conversation
for more information, see https://pre-commit.ci
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (10)
You can disable this status message by setting the 📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request focus on the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Model
participant Stats
User->>Model: Request predictions
Model->>Model: Process input with model_forward_auto_batch_size
Model-->>User: Return predictions
User->>Stats: Compute output statistics
Stats->>Stats: Reshape outputs and natoms
Stats-->>User: Return computed statistics
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
deepmd/pt/utils/stat.py (2)
551-551: Consider simplifying the reshaping ofnatomsfor clarityThe list comprehension in line 551:
natoms = {k: [sys_v.reshape(-1, 1) for sys_v in v] for k, v in natoms.items()}uses nested comprehensions, which can be difficult to read and maintain. Refactoring this line by introducing intermediate variables or using a loop can improve readability.
Consider refactoring as follows:
for k, v in natoms.items(): reshaped_list = [] for sys_v in v: reshaped_list.append(sys_v.reshape(-1, 1)) natoms[k] = reshaped_list
552-558: Refactor the nested comprehensions inoutputsfor better readabilityLines 552-558 involve nested dictionary and list comprehensions:
outputs = { k: [ sys.reshape(natoms[k][sys_idx].shape[0], 1, -1) for sys_idx, sys in enumerate(v) ] for k, v in outputs.items() }This complex nesting can make the code harder to understand and maintain. Consider breaking down the comprehensions into multiple steps or using helper functions to enhance clarity.
For example:
for k, v in outputs.items(): reshaped_list = [] for sys_idx, sys in enumerate(v): n_atoms = natoms[k][sys_idx].shape[0] reshaped_sys = sys.reshape(n_atoms, 1, -1) reshaped_list.append(reshaped_sys) outputs[k] = reshaped_list
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #4370 +/- ##
==========================================
+ Coverage 84.50% 84.54% +0.04%
==========================================
Files 596 597 +1
Lines 56665 56813 +148
Branches 3460 3487 +27
==========================================
+ Hits 47883 48033 +150
+ Misses 7654 7653 -1
+ Partials 1128 1127 -1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
source/tests/pt/water_tensor/dipole/O78H156/type.raw(1 hunks)source/tests/pt/water_tensor/dipole/O78H156/type_map.raw(1 hunks)source/tests/pt/water_tensor/dipole/O96H192/type.raw(1 hunks)source/tests/pt/water_tensor/dipole/O96H192/type_map.raw(1 hunks)source/tests/pt/water_tensor/se_e2_a.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- source/tests/pt/water_tensor/dipole/O78H156/type.raw
- source/tests/pt/water_tensor/dipole/O78H156/type_map.raw
- source/tests/pt/water_tensor/dipole/O96H192/type.raw
- source/tests/pt/water_tensor/dipole/O96H192/type_map.raw
🔇 Additional comments (1)
source/tests/pt/water_tensor/se_e2_a.json (1)
63-65: LGTM: Training dataset expansion looks good!
The addition of larger water systems (O78H156 and O96H192) to the training dataset will help improve the model's ability to handle varying system sizes.
Kept 2frames for each system. |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation