fix(dpa4): stabilize envelope-gated attention normalization#5808
Conversation
Normalize physical edge and null masses in one shifted frame so vanishing high-logit edges remain continuous without sacrificing mixed-precision stability.
Evaluate C3 envelopes without cancellation and decouple SFPG log scaling from its gradient path so cutoff attention and higher derivatives remain finite.
📝 WalkthroughWalkthroughThe change refactors C3 cutoff envelopes into factorized series evaluations, updates gated attention normalization with log-domain logits and physical null mass handling, adds numerical regression tests, and refreshes DPA4 example configurations and freeze guidance. ChangesNumerical stability and DPA4 updates
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@source/tests/pt/model/test_descriptor_sezm.py`:
- Around line 1937-1943: Strengthen the Hessian validation in the test around
attention_sum and physical_sum by adding a separate assertion for the
reference[0, 0] component, using a meaningful tolerance appropriate for its
approximately -0.25 value. Keep the existing full-matrix assert_close for the
remaining Hessian comparison.
- Around line 1857-1868: Extend the assertions in the test around
evaluate(1.0e-12) to verify that the affected tiny-positive edge remains
nonzero, not just the unaffected zero edge. Use the existing near tensor and
device/dtype conventions, while preserving the current expected_stable and
zero-edge assertions.
🪄 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: dd0e7c59-320e-43df-ad6e-378a7a98568b
📒 Files selected for processing (5)
deepmd/dpmodel/descriptor/dpa4_nn/attention.pydeepmd/dpmodel/descriptor/dpa4_nn/radial.pydeepmd/pt/model/descriptor/sezm_nn/attention.pydeepmd/pt/model/descriptor/sezm_nn/radial.pysource/tests/pt/model/test_descriptor_sezm.py
Assert the tiny positive attention mass and the small Hessian component so the cutoff tests cannot hide clamping or local derivative errors.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5808 +/- ##
==========================================
- Coverage 78.57% 78.31% -0.26%
==========================================
Files 1049 1050 +1
Lines 120650 120659 +9
Branches 4348 4358 +10
==========================================
- Hits 94806 94500 -306
- Misses 24285 24588 +303
- Partials 1559 1571 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
wanghan-iapcm
left a comment
There was a problem hiding this comment.
The numerics check out: the log-space softmax is algebraically equivalent with a correct log-sum-exp shift (the max-shift cancels through the null term, so autograd stays correct), the all-vanish case gives a well-defined alpha=0, the removed isfinite/eps guards were the original stabilization rather than a targeted patch and are provably unnecessary under the new null-mass formulation, and the C3 envelope factorization is exact (verified for p=1..7) and reaches exactly 0 at the cutoff. dpmodel and pt are faithful mirrors. LGTM.
One non-blocking follow-up on test coverage: the new extreme-regime tests (TestCutoffNumerics — nextafter-cutoff, src_weight=1e-30, high-logit vanishing, tiny-source-weight Hessian) are written only against the pt implementation. The dpmodel implementation is a separate rewrite and is the source of truth for the pt_expt and jax backends (pt_expt's descriptor subclasses this C3CutoffEnvelope), yet the existing cross-backend parity test (test_dpa4_dpmodel_parity.py) only feeds moderate rng.normal() values and never reaches these underflow/near-cutoff extremes. So nothing currently proves the dpmodel branch is equally stable in the exact regime this PR targets. Consider mirroring these edge cases into the parity file or adding a dpmodel-side equivalent, so the stability guarantee holds for all backends that consume dpmodel.
Minor: the Step 1 comment in deepmd/dpmodel/descriptor/dpa4_nn/attention.py lists edge_mask in the "edge_env**2 * src_weight * edge_mask underflows" product, but edge_mask is a binary 0/1 gate handled purely via the active predicate (never folded into log_weight), as the adjacent comment and the pt version correctly state. Worth aligning the wording.
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 `@source/tests/pt/model/test_dpa4_dpmodel_parity.py`:
- Around line 2339-2361: Update the cutoff-attention test around dp_softmax and
pt_softmax to use a larger logit that keeps the expected attention weight in the
normal float32 range. Replace the current range-only assertion with an explicit
comparison against the expected physical value, while preserving the existing
DP/PyTorch parity check.
🪄 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: 5aa0366e-ab43-479a-8bcd-bcf800caaa01
📒 Files selected for processing (2)
deepmd/dpmodel/descriptor/dpa4_nn/attention.pysource/tests/pt/model/test_dpa4_dpmodel_parity.py
🚧 Files skipped from review as they are similar to previous changes (1)
- deepmd/dpmodel/descriptor/dpa4_nn/attention.py
Summary by CodeRabbit
Bug Fixes
Tests
Documentation