test(pt): document se_e3_tebd gr output#5536
Conversation
The se_e3_tebd descriptor contracts three-body angular information into a rotationally invariant descriptor, so it does not expose a separate equivariant gr representation. Make the test assert this behavior explicitly and update the return-value docs. Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)
📝 WalkthroughWalkthroughThe ChangesClarify and enforce gr=None for se_t_tebd descriptor
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deepmd/dpmodel/descriptor/se_t_tebd.py (1)
363-363:⚠️ Potential issue | 🟠 MajorFix return type annotation to match actual 5-tuple return.
The return type annotation declares
-> tuple[Array, Array](2-tuple) at line 363, but the function returns a 5-tuple(grrg, rot_mat, None, None, sw)at line 420. The docstring correctly documents all 5 return values. Update the annotation totuple[Array, Array, None, None, Array]or use an appropriate type alias to match the implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deepmd/dpmodel/descriptor/se_t_tebd.py` at line 363, The return type annotation for the function declares a 2-tuple type `tuple[Array, Array]` but the actual return statement at line 420 returns a 5-tuple containing `(grrg, rot_mat, None, None, sw)`. Update the return type annotation to accurately reflect the 5-tuple being returned, changing it to `tuple[Array, Array, None, None, Array]` to match both the implementation and what is documented in the docstring.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@deepmd/dpmodel/descriptor/se_t_tebd.py`:
- Line 363: The return type annotation for the function declares a 2-tuple type
`tuple[Array, Array]` but the actual return statement at line 420 returns a
5-tuple containing `(grrg, rot_mat, None, None, sw)`. Update the return type
annotation to accurately reflect the 5-tuple being returned, changing it to
`tuple[Array, Array, None, None, Array]` to match both the implementation and
what is documented in the docstring.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d680dfba-0862-429e-a859-2de54d5a1f78
📒 Files selected for processing (2)
deepmd/dpmodel/descriptor/se_t_tebd.pysource/tests/pt_expt/descriptor/test_se_t_tebd.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5536 +/- ##
==========================================
+ Coverage 81.34% 82.18% +0.84%
==========================================
Files 868 890 +22
Lines 96358 101357 +4999
Branches 4233 4243 +10
==========================================
+ Hits 78383 83304 +4921
- Misses 16675 16751 +76
- Partials 1300 1302 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Problem
grisNone.gr, even though se_e3_tebd contracts the angular information into an invariant descriptor.Change
gr is Nonein the test.gris intentionallyNone.Notes
python3 -m py_compile source/tests/pt_expt/descriptor/test_se_t_tebd.py deepmd/dpmodel/descriptor/se_t_tebd.py;git diff --check.pytestinstalled.Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)
Summary by CodeRabbit
Documentation
Tests