pd: Ignore if branch of 0-size #4617
Conversation
📝 WalkthroughWalkthroughThe pull request modifies several control flow conditions within the DeepMD PD code. In the Changes
Sequence Diagram(s)sequenceDiagram participant Caller as Caller participant Desc as DescrptBlockSeA.forward participant LN as LayerNorm.forward Caller->>Desc: Invoke forward(input) Note right of Desc: Bypass conditional check based on new logic Desc->>Desc: Multiply rr with mm and extract ss Desc->>Caller: Return descriptor output Caller->>LN: Invoke forward(input) Note right of LN: Bypass conditional check based on new logic LN->>LN: Compute variance and mean LN->>Caller: Return normalized output sequenceDiagram participant Caller as Caller participant NList as NeighborList Function Caller->>NList: Call with coordinates and rcut Note right of NList: Bypass original condition based on new logic NList->>NList: Compute xmax using maximum value plus offset NList->>Caller: Return neighbor list with computed xmax Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (20)
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
🧹 Nitpick comments (4)
deepmd/pd/model/descriptor/se_a.py (1)
747-749: Changed tensor element check to unconditional executionThis change bypasses the original check for empty tensors, which is necessary because of limitations in paddle.jit's handling of control flow with double backward computation.
Note that there's a small typo in the comment - "ontrol" should be "control". This appears in all 3 files.
- # NOTE: ontrol flow with double backward is not supported well yet by paddle.jit + # NOTE: control flow with double backward is not supported well yet by paddle.jitdeepmd/pd/model/network/layernorm.py (1)
102-104: Changed tensor element check to unconditional executionThis change replaces the tensor element count check with an unconditional execution to work around limitations in paddle.jit's handling of control flow during double backward passes. Same approach as in the other files.
The same typo appears in the comment - "ontrol" should be "control".
- # NOTE: ontrol flow with double backward is not supported well yet by paddle.jit + # NOTE: control flow with double backward is not supported well yet by paddle.jitdeepmd/pd/utils/nlist.py (2)
101-103: Changed tensor element check to unconditional executionSimilar to the other files, this change replaces a check for empty tensors with an unconditional execution to avoid paddle.jit limitations with control flow during double backward passes.
Same comment typo - "ontrol" should be "control".
- # NOTE: ontrol flow with double backward is not supported well yet by paddle.jit + # NOTE: control flow with double backward is not supported well yet by paddle.jit
246-248: Changed tensor element check to unconditional executionThis change follows the same pattern as the others, replacing a tensor element count check with an unconditional block to avoid paddle.jit limitations with control flow during double backward passes.
Same typo in comment - "ontrol" should be "control".
- # NOTE: ontrol flow with double backward is not supported well yet by paddle.jit + # NOTE: control flow with double backward is not supported well yet by paddle.jit
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
deepmd/pd/model/descriptor/se_a.py(1 hunks)deepmd/pd/model/network/layernorm.py(1 hunks)deepmd/pd/utils/nlist.py(2 hunks)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
HydrogenSulfate force-pushed the workaround_for_control_flow branch from bd18b76 to f3489f1 Compare HydrogenSulfate force-pushed the workaround_for_control_flow branch from f3489f1 to 5d3599c Compare 
for more information, see https://pre-commit.ci
empty commit Signed-off-by: HydrogenSulfate <490868991@qq.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@ ## devel #4617 +/- ## ========================================== - Coverage 84.58% 84.58% -0.01% ========================================== Files 680 680 Lines 64547 64548 +1 Branches 3539 3540 +1 ========================================== Hits 54600 54600 - Misses 8806 8807 +1 Partials 1141 1141 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
The support for higher-order differentiation and complex control flow in
paddle.jitstatic graphs is not very strong. Therefore, branches related to 0-size are ignored to avoid accuracy issues during training.Summary by CodeRabbit