fix(pd): correct tensor reshaping and indexing #4827
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses tensor reshaping and indexing issues in the Paddle-based code.
- Replaces the chained tensor method call with a direct call to paddle.where for bin_count initialization in aggregate.
- Introduces a new boolean parameter (use_loc_mapping) in get_graph_index to adjust the frame shift computation.
- Updates reshape calls and modifies how dimensions are extracted in repflow_layer to align tensor operations correctly.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| deepmd/pd/model/network/utils.py | Uses paddle.where for bin_count and adds use_loc_mapping to control frame shift logic. |
| deepmd/pd/model/descriptor/repflow_layer.py | Updates reshape calls for clarity and changes dimension extraction in forward for indexing. |
Comments suppressed due to low confidence (2)
deepmd/pd/model/network/utils.py:105
- Ensure that the conditional logic for computing frame_shift using 'use_loc_mapping' is clearly documented in the function's docstring, as its behavior significantly affects tensor indexing.
nall if not use_loc_mapping else nloc
deepmd/pd/model/descriptor/repflow_layer.py:749
- Changing the dimension unpacking source from 'edge_ebd' to 'nlist' may lead to misaligned dimensions; verify that nlist.shape reliably provides nb, nloc, and nnei as intended.
nb, nloc, nnei = nlist.shape
for more information, see https://pre-commit.ci
📝 WalkthroughWalkthroughThe changes update tensor manipulation calls to ensure correct argument formatting and improve logic consistency. Shape arguments in Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Utils as utils.py
Caller->>Utils: get_graph_index(nlist, nlist_mask, a_nlist_mask, nall, use_loc_mapping)
alt use_loc_mapping is True
Utils->>Utils: frame_shift = frame * nloc
else use_loc_mapping is False
Utils->>Utils: frame_shift = frame * nall
end
Utils-->>Caller: graph_index
sequenceDiagram
participant Caller
participant Utils as utils.py
Caller->>Utils: aggregate(x, bin_index, bin_count)
Utils->>Utils: bin_count = paddle.where(bin_count == 0, 1, bin_count)
Utils-->>Caller: aggregated_result
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (3.3.7)deepmd/pd/model/descriptor/repflow_layer.pyNo files to lint: exiting. deepmd/pd/model/network/utils.pyNo files to lint: exiting. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsdeepmd/pd/model/network/utils.py (2)⏰ Context from checks skipped due to timeout of 90000ms (29)
🔇 Additional comments (7)
✨ 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 (
|
|
Thanks, I have made other PR for full modification, I guess this PR can be closed for better review. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #4827 +/- ##
==========================================
- Coverage 84.57% 84.57% -0.01%
==========================================
Files 699 699
Lines 68036 68035 -1
Branches 3540 3540
==========================================
- Hits 57540 57539 -1
+ Misses 9361 9360 -1
- Partials 1135 1136 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I've encountered some minor problems while trying out Paddle.
Summary by CodeRabbit
Bug Fixes
New Features