docs: add theory section to DPA3 documentation#5262
docs: add theory section to DPA3 documentation#5262iProzd merged 8 commits intodeepmodeling:masterfrom
Conversation
Add a comprehensive theory section to the DPA3 descriptor documentation, including: - Line Graph Series (LiGS) construction and geometric interpretation - Message passing mechanism with residual connections - Descriptor construction and energy prediction formulas - Physical symmetries (translational, rotational, permutational invariance and energy conservation) - Default configuration explanation Authored by OpenClaw (model: glm-5)
for more information, see https://pre-commit.ci
|
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:
📝 WalkthroughWalkthroughRewrote and expanded the DPA3 documentation: added a Theory section defining LiGS (Line Graph Series), formalized per-layer message-passing (vertex/edge updates and residual coupling), detailed descriptor and energy construction with symmetry proofs, updated default K=2 config and expanded hyperparameter experiments/results across six DFT datasets. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
doc/model/dpa3.md (1)
38-53: Consider noting how$G^{(1)}$ edge features are updated.The section presents the
$G^{(1)}$ vertex update (line 41) and the$G^{(k)}$ edge update for$k > 1$ (line 52), but never explicitly states that$G^{(1)}$ edge features are updated — they are, implicitly, because they equal the$G^{(2)}$ vertex features (by the identity), which in turn are driven by the$G^{(2)}$ edge update. Adding one sentence clarifying this would close the loop for readers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/model/dpa3.md` around lines 38 - 53, Add a clarifying sentence stating that G^{(1)} edge features are updated: explicitly say that the edge features \mathbf{e}_{\alpha\beta}^{(1,l)} are updated by the same Update mechanism (and therefore change across layers) and that, via the identity \mathbf{v}_\alpha^{(k,l)} = \mathbf{e}_{\alpha}^{(k-1,l)} (with k=2), G^{(1)} edges drive G^{(2)} vertex updates—this closes the loop between \mathbf{e}_{\alpha\beta}^{(1,l)}, the update functions Update^{(k)}, and the vertex identity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@doc/model/dpa3.md`:
- Around line 88-91: The statement claiming "K=2 … provides optimal balance
between accuracy and computational cost" for DPA3 is unsupported by the
hyperparameter table; either add a citation to the source (e.g., the ablation
study in the reference paper) that justifies K=2, or change the sentence to a
softer, non-empirical phrasing (e.g., "we use K=2 by default" or "K=2 was found
effective in prior work") so it no longer implies an unsupported experiment;
update the paragraph referencing DPA3, K, and the Default Configuration section
accordingly and include the citation identifier or revised wording next to the
K=2 claim.
- Line 80: Replace the incorrect phrase in the "Permutational invariance" bullet
— remove "respecting quantum statistics" and instead state that permutational
invariance means identical atoms are treated identically so the potential energy
is invariant under re-labeling (permutation symmetry of identical species);
update the sentence under the "Permutational invariance" heading accordingly and
mirror the phrasing used in the referenced paper ("permutation symmetry
operations") for clarity.
- Around line 44-48: The equation v_α^{(k,l)} = e_α^{(k-1,l)} uses a single
subscript for an edge but the document defines edges as e_{αβ}^{(k,l)}; fix by
replacing the RHS with the double-indexed edge e_{αβ}^{(k-1,l)} and add a short
clarifying phrase that (α,β) is the edge in G^{(k-1)} corresponding to vertex α
in G^{(k)} (or, if α was intended as an edge index, explicitly state that
mapping). Ensure the symbols match the earlier definitions (v_α^{(k,l)} and
e_{αβ}^{(k-1,l)}) and mirror the wording used at line 52 for consistency.
---
Nitpick comments:
In `@doc/model/dpa3.md`:
- Around line 38-53: Add a clarifying sentence stating that G^{(1)} edge
features are updated: explicitly say that the edge features
\mathbf{e}_{\alpha\beta}^{(1,l)} are updated by the same Update mechanism (and
therefore change across layers) and that, via the identity
\mathbf{v}_\alpha^{(k,l)} = \mathbf{e}_{\alpha}^{(k-1,l)} (with k=2), G^{(1)}
edges drive G^{(2)} vertex updates—this closes the loop between
\mathbf{e}_{\alpha\beta}^{(1,l)}, the update functions Update^{(k)}, and the
vertex identity.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
doc/model/dpa3.md (3)
101-101:⚠️ Potential issue | 🟡 MinorUnsupported optimality claim for K=2 (already flagged).
The hyperparameter table contains no rows varying
$K$ , so "provides optimal balance between accuracy and computational cost" is an unsubstantiated claim. Either cite the ablation study from the DPA3 paper or soften to: "K=2 is used by default, as found effective in prior work [DPA3 paper]."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/model/dpa3.md` at line 101, The sentence claiming "provides optimal balance between accuracy and computational cost" for LiGS order K=2 is unsupported by the presented hyperparameter table; change the wording to a neutral statement (e.g., "K=2 is used by default, as found effective in prior work [DPA3 paper]") or add a citation to the DPA3 ablation study that explicitly compares K values; update the sentence referencing "LiGS order K=2" and ensure any supporting evidence is added to the hyperparameter table or replaced with a citation to the paper.
89-89:⚠️ Potential issue | 🟡 Minor"Respecting quantum statistics" is incorrect terminology for permutational invariance (already flagged).
Permutational invariance in MLIPs is a classical symmetry of the PES under re-labeling of identical atoms; "quantum statistics" (Bose–Einstein/Fermi–Dirac) is an unrelated quantum-mechanical concept.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/model/dpa3.md` at line 89, The phrase "respecting quantum statistics" in the Permutational invariance sentence is incorrect; update the line that currently reads "Atoms of the same chemical species are treated identically, respecting quantum statistics." to something like "Atoms of the same chemical species are treated identically under permutations of atom indices" or "Atoms of the same chemical species are treated identically under re-labeling (permutations) of identical atoms," thereby removing the quantum-statistics wording and clarifying the classical symmetry of permutational invariance.
49-51:⚠️ Potential issue | 🟡 MinorNotation inconsistency: single subscript on edge feature (already flagged).
The identity $\mathbf{v}\alpha^{(k,l)} = \mathbf{e}{\alpha}^{(k-1,l)}$ uses a single subscript for an edge feature, which conflicts with the double-indexed convention
$\mathbf{e}_{\alpha\beta}^{(k,l)}$ established at Line 35.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/model/dpa3.md` around lines 49 - 51, The equation uses a single-index edge feature e_{α}^{(k-1,l)} which conflicts with the established double-indexed edge notation e_{αβ}^{(k,l)}; update the identity to use the double-index form (e.g., v_{α}^{(k,l)} = e_{αβ}^{(k-1,l)} with the appropriate β specified or a clarifying statement that β is the neighbor index) and/or add a short note defining the shorthand if you intended a summed/implicit β; ensure you update the symbols v_{α}^{(k,l)}, e_{αβ}^{(k-1,l)} to be consistent with the notation introduced at Line 35.
🧹 Nitpick comments (1)
doc/model/dpa3.md (1)
72-72:$\oplus$ notation is undefined.
$\oplus$ conventionally denotes XOR or direct sum; using it for concatenation without a definition may confuse readers. Either define it inline or use a more standard concatenation notation.📝 Proposed fix
-E_i = \mathcal{N}_{\text{fit}}(\mathcal{D}^i \oplus \mathbf{d}_{\text{dataset}}) +E_i = \mathcal{N}_{\text{fit}}([\mathcal{D}^i \| \mathbf{d}_{\text{dataset}}])or add a note: "where
$\oplus$ denotes concatenation."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/model/dpa3.md` at line 72, The expression uses an undefined ⊕ operator (E_i = \mathcal{N}_{\text{fit}}(\mathcal{D}^i ⊕ \mathbf{d}_{\text{dataset}})); update the text to either (a) replace ⊕ with an explicit concatenation notation (e.g., \mathbin{\|} or \concat) or (b) add a short inline definition such as "where ⊕ denotes concatenation of datasets" immediately after the equation so readers know how \mathcal{D}^i and \mathbf{d}_{\text{dataset}} are being combined when passed to \mathcal{N}_{\text{fit}}.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@doc/model/dpa3.md`:
- Around line 61-65: The document uses α as the vertex index in the Message
Passing section but then introduces i in the descriptor definition (𝒟^i =
v_i^{(1,L)}); make the index consistent by replacing i with α (i.e., 𝒟^α =
v_α^{(1,L)}) everywhere or explicitly state “let i ≡ α” and update all
subsequent references (including the descriptor 𝒟 and any uses in the following
equations/paragraphs) to match that choice so the symbol for the atom/vertex
index is uniform (refer to 𝒟^i, v_i^{(1,L)}, and α).
- Line 35: The notation treats d_v and d_e as global feature dims but the model
actually uses per-graph dimensions that map to RepFlowArgs (n_dim, e_dim, a_dim)
and obey identities like v_alpha^{(k,l)} = e_alpha^{(k-1,l)} for k>1; update the
text to use per-graph dimension symbols (e.g. d_v^{(k)}, d_e^{(k)}) or add an
explicit mapping paragraph stating d_v^{(1)} = n_dim, d_e^{(1)} = e_dim,
d_v^{(2)} = e_dim, d_e^{(2)} = a_dim (and generalize for further k if needed)
and mention RepFlowArgs to tie the math to implementation.
- Line 87: Update the inaccurate description: replace the sentence claiming the
descriptor is "constructed from scalar features that are invariant under global
rotations" with wording that clarifies intermediate equivariant representations
are used and then contracted to an invariant descriptor; reference the DPA3
implementation (deepmd/dpmodel/descriptor/dpa3.py) and its intermediate shape
"nf x nloc x e_dim x 3" to note the ×3 vector components are equivariant, and
use phrasing such as "The descriptor is rotationally invariant; intermediate
equivariant representations are used internally and contracted to produce the
final invariant atomic features."
---
Duplicate comments:
In `@doc/model/dpa3.md`:
- Line 101: The sentence claiming "provides optimal balance between accuracy and
computational cost" for LiGS order K=2 is unsupported by the presented
hyperparameter table; change the wording to a neutral statement (e.g., "K=2 is
used by default, as found effective in prior work [DPA3 paper]") or add a
citation to the DPA3 ablation study that explicitly compares K values; update
the sentence referencing "LiGS order K=2" and ensure any supporting evidence is
added to the hyperparameter table or replaced with a citation to the paper.
- Line 89: The phrase "respecting quantum statistics" in the Permutational
invariance sentence is incorrect; update the line that currently reads "Atoms of
the same chemical species are treated identically, respecting quantum
statistics." to something like "Atoms of the same chemical species are treated
identically under permutations of atom indices" or "Atoms of the same chemical
species are treated identically under re-labeling (permutations) of identical
atoms," thereby removing the quantum-statistics wording and clarifying the
classical symmetry of permutational invariance.
- Around line 49-51: The equation uses a single-index edge feature
e_{α}^{(k-1,l)} which conflicts with the established double-indexed edge
notation e_{αβ}^{(k,l)}; update the identity to use the double-index form (e.g.,
v_{α}^{(k,l)} = e_{αβ}^{(k-1,l)} with the appropriate β specified or a
clarifying statement that β is the neighbor index) and/or add a short note
defining the shorthand if you intended a summed/implicit β; ensure you update
the symbols v_{α}^{(k,l)}, e_{αβ}^{(k-1,l)} to be consistent with the notation
introduced at Line 35.
---
Nitpick comments:
In `@doc/model/dpa3.md`:
- Line 72: The expression uses an undefined ⊕ operator (E_i =
\mathcal{N}_{\text{fit}}(\mathcal{D}^i ⊕ \mathbf{d}_{\text{dataset}})); update
the text to either (a) replace ⊕ with an explicit concatenation notation (e.g.,
\mathbin{\|} or \concat) or (b) add a short inline definition such as "where ⊕
denotes concatenation of datasets" immediately after the equation so readers
know how \mathcal{D}^i and \mathbf{d}_{\text{dataset}} are being combined when
passed to \mathcal{N}_{\text{fit}}.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5262 +/- ##
=======================================
Coverage 82.00% 82.00%
=======================================
Files 750 750
Lines 75082 75080 -2
Branches 3615 3616 +1
=======================================
+ Hits 61571 61572 +1
+ Misses 12347 12345 -2
+ Partials 1164 1163 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Authored by OpenClaw (model: glm-5)
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
🧹 Nitpick comments (3)
doc/model/dpa3.md (3)
43-57: Optional: disambiguateUpdate^{(k)}for vertex vs. edge updates.
Update^{(1)}in line 43 (vertex aggregation over a set of edge features) andUpdate^{(k)}in line 57 (edge update from two vertex features) have incompatible signatures. For k=1 both equations carry the same superscript, which can mislead a reader into thinking they refer to the same parameterized module.Consider distinguishing them with subscripts:
📝 Suggested notation clarification
-\mathbf{v}_\alpha^{(1,l+1)} = \mathbf{v}_\alpha^{(1,l)} + \text{Update}^{(1)}\left(\mathbf{v}_\alpha^{(1,l)}, \{\mathbf{e}_{\alpha\beta}^{(1,l)}\}_{\beta \in \mathcal{N}(\alpha)}\right) +\mathbf{v}_\alpha^{(1,l+1)} = \mathbf{v}_\alpha^{(1,l)} + \text{Update}_V^{(1)}\left(\mathbf{v}_\alpha^{(1,l)}, \{\mathbf{e}_{\alpha\beta}^{(1,l)}\}_{\beta \in \mathcal{N}(\alpha)}\right)-\mathbf{e}_{\alpha\beta}^{(k,l+1)} = \mathbf{e}_{\alpha\beta}^{(k,l)} + \text{Update}^{(k)}\left(\mathbf{e}_{\alpha\beta}^{(k,l)}, \mathbf{v}_\alpha^{(k,l)}, \mathbf{v}_\beta^{(k,l)}\right) +\mathbf{e}_{\alpha\beta}^{(k,l+1)} = \mathbf{e}_{\alpha\beta}^{(k,l)} + \text{Update}_E^{(k)}\left(\mathbf{e}_{\alpha\beta}^{(k,l)}, \mathbf{v}_\alpha^{(k,l)}, \mathbf{v}_\beta^{(k,l)}\right)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/model/dpa3.md` around lines 43 - 57, The notation reuses Update^{(k)} for different signatures (vertex vs edge updates) which is confusing; change symbols to explicitly distinguish the two update modules (e.g., use Update_V^{(1)} or Update_vertex^{(1)} for the vertex aggregation in the v_alpha update and use Update_E^{(k)} or Update_edge^{(k)} for the edge update e_{alpha beta}), update the text and all equations that reference Update^{(1)} and Update^{(k)} accordingly, and add a short parenthetical or sentence defining each symbol’s signature (e.g., Update_vertex(v, {e_i}) and Update_edge(e, v_i, v_j)) so readers can see the different parameterizations at a glance.
88-97: Optional: use$\alpha$ consistently for atom indices in the Physical Symmetries section.Lines 88 and 97 silently switch to
$i, j$ notation. The rest of the Theory section establishes$\alpha, \beta$ as atom/vertex indices (a past review fixed the same inconsistency in the Descriptor Construction section). While$i, j$ is conventional physics notation, keeping$\alpha$ here maintains document-wide consistency.📝 Suggested change
-1. **Translational invariance**: The model depends only on relative coordinates $\mathbf{r}_{ij} = \mathbf{r}_j - \mathbf{r}_i$, not absolute positions. +1. **Translational invariance**: The model depends only on relative coordinates $\mathbf{r}_{\alpha\beta} = \mathbf{r}_\beta - \mathbf{r}_\alpha$, not absolute positions.-\mathbf{F}_i = -\frac{\partial E}{\partial \mathbf{r}_i} +\mathbf{F}_\alpha = -\frac{\partial E}{\partial \mathbf{r}_\alpha}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/model/dpa3.md` around lines 88 - 97, The Physical Symmetries bullets switch to i,j notation; update them to use the document's established atom indices (e.g., α, β) for consistency—replace r_{ij} with r_{αβ} (and r_j - r_i with r_β - r_α), change F_i to F_α and ∂/∂r_i to ∂/∂r_α, and ensure any mention of "atoms i,j" uses "atoms α,β" so the Translational invariance, Rotational invariance, Permutational invariance, and Energy conservation lines consistently use the α/β indexing.
94-100: Optional: "Energy conservation" is a force property, not a symmetry — consider restructuring.The section header states "DPA3 respects all physical symmetries", but items 1–3 are genuine symmetries (translational, rotational, permutational) while item 4 — energy conservation via gradient-derived forces — is a property of conservative force fields, not a symmetry in the Noether/group-theory sense. This introduces a subtle categorical mismatch.
📝 Suggested fix
-### Physical Symmetries +### Physical Symmetries and Conservative Forces DPA3 respects all physical symmetries of the potential energy surface: 1. **Translational invariance**: ... 1. **Rotational invariance**: ... 1. **Permutational invariance**: ... +In addition, forces and virials are derived from energy gradients, making the model conservative: + 1. **Energy conservation**: Forces are derived from energy gradients:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/model/dpa3.md` around lines 94 - 100, The "Energy conservation" item (the paragraph containing "Energy conservation: Forces are derived from energy gradients" and the equation F_i = -∂E/∂r_i) is a property of conservative force fields, not a physical symmetry; update the document by either (A) changing the section header "DPA3 respects all physical symmetries" to something like "DPA3: physical symmetries and properties" and leave the energy-conservation item in the list, or (B) move that paragraph into a new "Properties" or "Conservative dynamics" subsection immediately after the symmetry list and adjust the surrounding text to reference it (mention the equation F_i = -∂E/∂r_i remains as the explanation). Ensure cross-references and numbering reflect the new placement and that the symmetry list contains only translational, rotational, and permutational items.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@doc/model/dpa3.md`:
- Around line 43-57: The notation reuses Update^{(k)} for different signatures
(vertex vs edge updates) which is confusing; change symbols to explicitly
distinguish the two update modules (e.g., use Update_V^{(1)} or
Update_vertex^{(1)} for the vertex aggregation in the v_alpha update and use
Update_E^{(k)} or Update_edge^{(k)} for the edge update e_{alpha beta}), update
the text and all equations that reference Update^{(1)} and Update^{(k)}
accordingly, and add a short parenthetical or sentence defining each symbol’s
signature (e.g., Update_vertex(v, {e_i}) and Update_edge(e, v_i, v_j)) so
readers can see the different parameterizations at a glance.
- Around line 88-97: The Physical Symmetries bullets switch to i,j notation;
update them to use the document's established atom indices (e.g., α, β) for
consistency—replace r_{ij} with r_{αβ} (and r_j - r_i with r_β - r_α), change
F_i to F_α and ∂/∂r_i to ∂/∂r_α, and ensure any mention of "atoms i,j" uses
"atoms α,β" so the Translational invariance, Rotational invariance,
Permutational invariance, and Energy conservation lines consistently use the α/β
indexing.
- Around line 94-100: The "Energy conservation" item (the paragraph containing
"Energy conservation: Forces are derived from energy gradients" and the equation
F_i = -∂E/∂r_i) is a property of conservative force fields, not a physical
symmetry; update the document by either (A) changing the section header "DPA3
respects all physical symmetries" to something like "DPA3: physical symmetries
and properties" and leave the energy-conservation item in the list, or (B) move
that paragraph into a new "Properties" or "Conservative dynamics" subsection
immediately after the symmetry list and adjust the surrounding text to reference
it (mention the equation F_i = -∂E/∂r_i remains as the explanation). Ensure
cross-references and numbering reflect the new placement and that the symmetry
list contains only translational, rotational, and permutational items.
Authored by OpenClaw (model: glm-5)
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
doc/model/dpa3.md (2)
94-100: Optional: "Energy conservation" heading can be confused with the physical law.The content describes a conservative force field (forces are gradients of energy), not the physical principle of energy conservation over time. The paper's own phrasing — "the DPA3 model is inherently conservative" — is more precise. Consider renaming to "Conservative force field" to avoid ambiguity, which is also consistent with line 100's "ensuring the model is conservative."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/model/dpa3.md` around lines 94 - 100, The heading "Energy conservation" is ambiguous; rename it to "Conservative force field" and update the paragraph so it states that forces are derived as gradients of the potential energy (e.g., \mathbf{F}_i = -\partial E/\partial \mathbf{r}_i) and that virials come from gradients with respect to the cell tensor, matching the paper's phrasing "the DPA3 model is inherently conservative" and keeping the same equations (\mathbf{F}_i, E) and mention of virials/cell tensor gradients.
39-60: The G^(1) edge update rule is orphaned by the section structure and shares ambiguous notation with the vertex update.Two related problems:
Structural scope: The edge update equation (line 57) appears under the "For G^(k) with k>1" heading, yet line 60 states the same equation applies to G^(1) edges. A reader following the sectioning will conclude that G^(1) edges are static — the retroactive prose patch on line 60 is easy to miss.
Notation clash:
Update^{(k)}in line 57 (edge update) is indistinguishable by superscript fromUpdate^{(1)}in line 43 (vertex update) when k=1. Adding a subscript (e.g.$\text{Update}_v^{(k)}$ vs$\text{Update}_e^{(k)}$ ) or restructuring the section would remove ambiguity.♻️ Suggested restructuring
-**For $G^{(1)}$ (initial graph):** -The vertex features are updated through self-message and symmetrization: +**For $G^{(k)}$ (all graphs, edge update):** +The edge features are updated based on messages from connected vertices: ```math -\mathbf{v}_\alpha^{(1,l+1)} = \mathbf{v}_\alpha^{(1,l)} + \text{Update}^{(1)}\left(\mathbf{v}_\alpha^{(1,l)}, \{\mathbf{e}_{\alpha\beta}^{(1,l)}\}_{\beta \in \mathcal{N}(\alpha)}\right) +\mathbf{e}_{\alpha\beta}^{(k,l+1)} = \mathbf{e}_{\alpha\beta}^{(k,l)} + \text{Update}_e^{(k)}\left(\mathbf{e}_{\alpha\beta}^{(k,l)}, \mathbf{v}_\alpha^{(k,l)}, \mathbf{v}_\beta^{(k,l)}\right)-For
$G^{(k)}$ with$k > 1$ :
-The vertex feature of$G^{(k)}$ is identical to the edge feature of$G^{(k-1)}$ . ...
+For$G^{(1)}$ (initial graph, vertex update):
+The vertex features are updated through self-message and symmetrization:-\mathbf{v}_\alpha^{(k,l)} = \mathbf{e}_{\alpha\beta}^{(k-1,l)} +\mathbf{v}_\alpha^{(1,l+1)} = \mathbf{v}_\alpha^{(1,l)} + \text{Update}_v^{(1)}\left(\mathbf{v}_\alpha^{(1,l)}, \{\mathbf{e}_{\alpha\beta}^{(1,l)}\}_{\beta \in \mathcal{N}(\alpha)}\right)-The edge features are updated based on messages from connected vertices:
-
math -\mathbf{e}_{\alpha\beta}^{(k,l+1)} = \mathbf{e}_{\alpha\beta}^{(k,l)} + \text{Update}^{(k)}\left(\mathbf{e}_{\alpha\beta}^{(k,l)}, \mathbf{v}_\alpha^{(k,l)}, \mathbf{v}_\beta^{(k,l)}\right) --The same update mechanism applies to
$G^{(1)}$ edge features$\mathbf{e}_{\alpha\beta}^{(1,l)}$ , ...
+For$G^{(k)}$ with$k > 1$ (vertex identity):
+The vertex feature of$G^{(k)}$ is identical to the edge feature of$G^{(k-1)}$ . ...</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@doc/model/dpa3.mdaround lines 39 - 60, The section misplaces and
ambiguously notates the edge update: move the edge-update equation out from
under "For G^(k) with k > 1" into a clearly labeled "Edge updates" (or include
it in the G^(1) block) and disambiguate the update operators by renaming
Update^{(k)} for edges to a distinct symbol (e.g. Update_e^{(k)}) and the vertex
updater to Update_v^{(k)}; update the equations for v_alpha^{(1,l+1)} and
e_{alpha beta}^{(k,l+1)} to use Update_v^{(1)} and Update_e^{(k)} respectively
and add a short note that e^{(1,l)} also follows the same Update_e^{(1)} rule so
the v^{(2,l)}=e^{(1,l)} identity remains clear.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@doc/model/dpa3.md:
- Around line 96-98: The force equation uses subscript i (F_i = -∂E/∂r_i) but
the Theory section consistently indexes atoms with α; update the force equation
to use the same index symbol (replace every occurrence of i in that equation
with α: F_α = -∂E/∂r_α) so the notation is consistent with the other equations
(e.g., those using α earlier in the section).
Nitpick comments:
In@doc/model/dpa3.md:
- Around line 94-100: The heading "Energy conservation" is ambiguous; rename it
to "Conservative force field" and update the paragraph so it states that forces
are derived as gradients of the potential energy (e.g., \mathbf{F}_i = -\partial
E/\partial \mathbf{r}_i) and that virials come from gradients with respect to
the cell tensor, matching the paper's phrasing "the DPA3 model is inherently
conservative" and keeping the same equations (\mathbf{F}_i, E) and mention of
virials/cell tensor gradients.- Around line 39-60: The section misplaces and ambiguously notates the edge
update: move the edge-update equation out from under "For G^(k) with k > 1" into
a clearly labeled "Edge updates" (or include it in the G^(1) block) and
disambiguate the update operators by renaming Update^{(k)} for edges to a
distinct symbol (e.g. Update_e^{(k)}) and the vertex updater to Update_v^{(k)};
update the equations for v_alpha^{(1,l+1)} and e_{alpha beta}^{(k,l+1)} to use
Update_v^{(1)} and Update_e^{(k)} respectively and add a short note that
e^{(1,l)} also follows the same Update_e^{(1)} rule so the v^{(2,l)}=e^{(1,l)}
identity remains clear.</details> --- <details> <summary>ℹ️ Review info</summary> **Configuration used**: Repository UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 9d6b27cc5369d91ffbfd739ba176568d1ebfa4f9 and 7790cd4210376023511fa834ed9769eef69b2d7f. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `doc/model/dpa3.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
♻️ Duplicate comments (1)
doc/model/dpa3.md (1)
89-99:⚠️ Potential issue | 🟡 MinorIndex inconsistency:
iandjnotation in Physical Symmetries vsα/βused throughout Theory.Line 89 introduces
r_{ij}using atom indicesi,j, and line 98 usesF_i— both inconsistent withα/βestablished as the vertex (atom) indices throughout the rest of the Theory section (lines 35, 43, 50, 58, 68, 76, 82).📝 Proposed fix
-1. **Translational invariance**: The model depends only on relative coordinates $\mathbf{r}_{ij} = \mathbf{r}_j - \mathbf{r}_i$, not absolute positions. +1. **Translational invariance**: The model depends only on relative coordinates $\mathbf{r}_{\alpha\beta} = \mathbf{r}_\beta - \mathbf{r}_\alpha$, not absolute positions.-\mathbf{F}_i = -\frac{\partial E}{\partial \mathbf{r}_i} +\mathbf{F}_\alpha = -\frac{\partial E}{\partial \mathbf{r}_\alpha}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/model/dpa3.md` around lines 89 - 99, The Physical Symmetries section uses atom indices i and j (symbols r_{ij} and F_i) which conflicts with the α/β vertex index notation used throughout the Theory section; update the notation in this section to use α and β consistently (e.g., replace r_{ij} with r_{αβ} and F_i with F_α) and ensure any explanatory sentences or equations reference the same α/β symbols so the document uses a single atomic-index convention (check occurrences of r_{ij}, F_i, and any other i/j indices in this block and replace them with α/β).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@doc/model/dpa3.md`:
- Around line 89-99: The Physical Symmetries section uses atom indices i and j
(symbols r_{ij} and F_i) which conflicts with the α/β vertex index notation used
throughout the Theory section; update the notation in this section to use α and
β consistently (e.g., replace r_{ij} with r_{αβ} and F_i with F_α) and ensure
any explanatory sentences or equations reference the same α/β symbols so the
document uses a single atomic-index convention (check occurrences of r_{ij},
F_i, and any other i/j indices in this block and replace them with α/β).
Authored by OpenClaw (model: gpt-5.3-codex)
Authored by OpenClaw (model: gpt-5.3-codex)
njzjz
changed the title
3 participants
Add a comprehensive theory section to the DPA3 descriptor documentation, including:
This follows the style of other model documentation (e.g.,
se_e2_a.md) in the repository.Authored by OpenClaw (model: glm-5)
Summary by CodeRabbit