docs(dpmodel): document mathematical operations#5787
Conversation
Add NumPyDoc equations for environment construction, switching functions, neural layers, type embeddings, spherical harmonics, and loss objectives. Clarify normalization, masking, residual, and extensive-property conventions. Coding-Agent: Codex Codex-Version: codex-cli 0.144.1 Model: gpt-5.6-sol Reasoning-Effort: xhigh
Document mathematical mappings for atomic models, output models, fitting nets, attention layers, losses, and utility abstractions. Coding-Agent: Codex Codex-Version: codex-cli 0.144.1 Model: gpt-5.6-sol Reasoning-Effort: xhigh
njzjz
changed the title
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
🚧 Files skipped from review as they are similar to previous changes (12)
📝 WalkthroughWalkthroughThis documentation-only change adds or expands mathematical and behavioral docstrings across atomic models, descriptors, fitting and loss modules, models, and utility components. No runtime logic, control flow, method signatures, or public APIs changed. ChangesModel and fitting documentation
Descriptor documentation
Loss documentation
Utility and network documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@deepmd/dpmodel/atomic_model/base_atomic_model.py`:
- Around line 56-60: Update the class docstring for the base atomic model to
explicitly define \mathcal N_i as the atom-i neighbor environment, clarifying
whether it represents the neighbor list, coordinates, or complete local
environment; keep the existing mapping equation unchanged.
In `@deepmd/dpmodel/descriptor/dpa1.py`:
- Line 2401: Update the docstring near GatedAttentionLayer.call to describe only
the projected gated self-attention output, removing the H + residual equation.
Document the residual addition instead on NeighborGatedAttentionLayer.call,
where the H plus attention result is actually applied.
In `@deepmd/dpmodel/descriptor/repformers.py`:
- Around line 968-969: Update the equations in
deepmd/dpmodel/descriptor/repformers.py at lines 968-969, 1099-1100, and 1272 to
match the implemented attention pipeline: document masked and smoothed softmax
attention with angular weighting at lines 968-969 instead of raw logits,
document O = AV at lines 1099-1100 while noting softmax and preprocessing occur
in Atten2Map, and include /sqrt(hidden_dim) scaling plus optional smoothing and
masking in the local-attention equation at line 1272.
In `@deepmd/dpmodel/model/property_model.py`:
- Around line 34-35: Update the PropertyModel class docstring to distinguish
reduction behavior by property type: state that extensive properties use p=∑ᵢ
pᵢ, while intensive properties use the configured intensive reduction instead of
implying all properties are summed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4587d592-5c02-408b-972b-524be61b8cd4
📒 Files selected for processing (29)
deepmd/dpmodel/atomic_model/base_atomic_model.pydeepmd/dpmodel/atomic_model/dipole_atomic_model.pydeepmd/dpmodel/atomic_model/dos_atomic_model.pydeepmd/dpmodel/atomic_model/energy_atomic_model.pydeepmd/dpmodel/atomic_model/polar_atomic_model.pydeepmd/dpmodel/atomic_model/property_atomic_model.pydeepmd/dpmodel/descriptor/dpa1.pydeepmd/dpmodel/descriptor/dpa2.pydeepmd/dpmodel/descriptor/repflows.pydeepmd/dpmodel/descriptor/repformers.pydeepmd/dpmodel/fitting/dos_fitting.pydeepmd/dpmodel/fitting/ener_fitting.pydeepmd/dpmodel/loss/dos.pydeepmd/dpmodel/loss/ener.pydeepmd/dpmodel/loss/loss.pydeepmd/dpmodel/loss/property.pydeepmd/dpmodel/loss/tensor.pydeepmd/dpmodel/model/dipole_model.pydeepmd/dpmodel/model/dos_model.pydeepmd/dpmodel/model/dp_zbl_model.pydeepmd/dpmodel/model/polar_model.pydeepmd/dpmodel/model/property_model.pydeepmd/dpmodel/utils/env_mat.pydeepmd/dpmodel/utils/env_mat_stat.pydeepmd/dpmodel/utils/learning_rate.pydeepmd/dpmodel/utils/network.pydeepmd/dpmodel/utils/spherical_harmonics.pydeepmd/dpmodel/utils/type_embed.pydeepmd/dpmodel/utils/update_sel.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5787 +/- ##
==========================================
- Coverage 79.76% 79.58% -0.19%
==========================================
Files 1023 1022 -1
Lines 117463 117351 -112
Branches 4325 4311 -14
==========================================
- Hits 93691 93390 -301
- Misses 22229 22409 +180
- Partials 1543 1552 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Coding-Agent: Codex Codex-Version: codex-cli 0.144.1 Model: gpt-5.6-sol Reasoning-Effort: xhigh
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@deepmd/dpmodel/descriptor/dpa4.py`:
- Around line 172-180: Update the descriptor documentation around _apply_readout
to define \mathcal D_i as the scalar output after applying the configured
readout to \mathbf h_i^{(L)}, rather than the pre-readout \mathbf h_i^{(L,0,0)}.
Explain that the identity mode preserves the scalar coefficient, while "glu" and
"mlp" fold higher-degree coefficients into the l=0 output before extraction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: be83e5e2-6dbb-46c6-805d-78ff8007693f
📒 Files selected for processing (6)
deepmd/dpmodel/descriptor/dpa1.pydeepmd/dpmodel/descriptor/dpa3.pydeepmd/dpmodel/descriptor/dpa4.pydeepmd/dpmodel/descriptor/repformers.pydeepmd/dpmodel/fitting/dpa4_ener.pydeepmd/dpmodel/loss/ener_spin.py
🚧 Files skipped from review as they are similar to previous changes (2)
- deepmd/dpmodel/descriptor/dpa1.py
- deepmd/dpmodel/descriptor/repformers.py
Coding-Agent: Codex Codex-Version: codex-cli 0.144.1 Model: gpt-5.6-sol Reasoning-Effort: xhigh
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@deepmd/dpmodel/descriptor/dpa1.py`:
- Around line 2420-2433: Update the projected gated self-attention docstring
around the displayed Q/K/V equations to document optional query/key/value
normalization and use the implementation’s scaling_factor or temperature instead
of fixed 1/sqrt(d). Clarify that angular weights are applied after softmax,
while masks and cutoff smoothing affect the pre-softmax attention logits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f0936ec6-52c1-45e1-b0c8-8c4578f639a9
📒 Files selected for processing (5)
deepmd/dpmodel/atomic_model/base_atomic_model.pydeepmd/dpmodel/descriptor/dpa1.pydeepmd/dpmodel/descriptor/dpa4.pydeepmd/dpmodel/descriptor/repformers.pydeepmd/dpmodel/model/property_model.py
🚧 Files skipped from review as they are similar to previous changes (4)
- deepmd/dpmodel/model/property_model.py
- deepmd/dpmodel/atomic_model/base_atomic_model.py
- deepmd/dpmodel/descriptor/dpa4.py
- deepmd/dpmodel/descriptor/repformers.py
Coding-Agent: Codex Codex-Version: codex-cli 0.144.1 Model: gpt-5.6-sol Reasoning-Effort: xhigh
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
The new mathematical documentation currently misstates the ZBL model coupling as a direct sum. The implementation uses a coordinate-dependent switched interpolation, so this formula needs to be corrected before merging.
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One additional mathematical documentation issue in the DPA4 interaction equation:
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One additional DPA4 readout documentation issue:
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One additional attention-operator documentation issue:
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One additional DPA3 message-passing documentation issue:
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One additional equivariant-output documentation issue:
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One additional tensor-output documentation issue:
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One additional generated-API-documentation issue:
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One additional loss-normalization documentation issue:
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One additional base-class API documentation issue:
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One additional DPA1 attention-operator documentation issue:
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One additional generated fitting-API documentation regression:
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
One final low-priority return-semantics documentation issue:
Align the ZBL, attention, descriptor, equivariant-output, and loss documentation with the implemented operations. Preserve inherited fitting API parameter documentation for AutoAPI and numpydoc. Coding-Agent: Codex Codex-Version: codex-cli 0.144.1 Model: gpt-5.6-sol Reasoning-Effort: xhigh
|
Fixed the 13 unresolved review threads and pushed commit Main corrections:
Validation:
Coding agent: Codex |
wanghan-iapcm
left a comment
wanghan-iapcm
left a comment
There was a problem hiding this comment.
Docstring-only change; verified no executable code was added or removed. The math descriptions match the implementations (dipole/polar reconstruction, energy/DOS/property sums, gated-attention layer), and the fitting classes compose their equation onto the inherited base doc (+ InvarFitting.doc) so the numpydoc Parameters sections are preserved for AutoAPI. LGTM.
Dismissing to re-review through the /code-review skill per process.
wanghan-iapcm
left a comment
wanghan-iapcm
left a comment
There was a problem hiding this comment.
Re-reviewed via the code-review skill. Documentation-only (verified: all deletions are docstring rewrites; the doc = math + Base.doc compose pattern preserves the inherited numpydoc Parameters). Formulas checked against the implementations across atomic models, descriptors (dpa1 attention, dpa3 message-passing), fittings, losses, and utils -- no incorrect formulas (spherical harmonics verified numerically). Only cosmetic/minor sub-threshold notes (a Sphinx block-quote indentation in the dos/ener fitting doc concatenation, and a few formulas that omit a secondary term); non-blocking. LGTM.
|
The documentation corrections requested in the earlier review are already present on the current head; the review threads are resolved/outdated and the current diff remains documentation-only. Please re-review the current head when convenient. Coding agent: Codex |
8f298eb
5 participants
Summary by CodeRabbit