Skip to content

Mesh: standardize tensor-valued gradient layout - #1772

Merged
peterdsharpe merged 2 commits into
NVIDIA:mainfrom
peterdsharpe:codex/mesh-calculus-gradient-layout
Jul 9, 2026
Merged

peterdsharpe merged 2 commits into
NVIDIA:mainfrom
peterdsharpe:codex/mesh-calculus-gradient-layout

Conversation

@peterdsharpe

Copy link
Copy Markdown
Collaborator

PhysicsNeMo Pull Request

Description

Previously, tensor-valued LSQ gradients moved the derivative-coordinate axis to the end, while the documented API, the functional LSQ implementation, and DEC use the derivative-first layout. This made the result depend on the gradient implementation and made vector-field Jacobians easy to misinterpret.

This PR standardizes tensor-valued gradients on (entity, n_spatial_dims, *value_shape):

  • Use gradient[i, k, j] = ∂v_j/∂x_k for point and cell LSQ gradients.
  • Apply the same layout to intrinsic LSQ and DEC, including fields with arbitrary trailing dimensions.
  • Update curl indexing for the derivative-first Jacobian; divergence and curl values are unchanged.
  • Add regression coverage for point/cell, intrinsic/extrinsic, LSQ/DEC, divergence, curl, and stored derivative data.

This is an intentional compatibility change for tensor-valued LSQ gradients. Scalar gradients are unchanged. A stored legacy tensor gradient can be converted with legacy_gradient.movedim(-1, 1).

Checklist

Dependencies

Review Process

All PRs are reviewed by the PhysicsNeMo team before merging.

Depending on which files are changed, GitHub may automatically assign a maintainer for review.

We are also testing AI-based code review tools (e.g., Greptile), which may add automated comments with a confidence score.
This score reflects the AI’s assessment of merge readiness and is not a qualitative judgment of your work, nor is
it an indication that the PR will be accepted / rejected.

AI-generated feedback should be reviewed critically for usefulness.
You are not required to respond to every AI comment, but they are intended to help both authors and reviewers.
Please react to Greptile comments with 👍 or 👎 to provide feedback on their accuracy.

@copy-pr-bot

copy-pr-bot Bot commented Jul 1, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@peterdsharpe

Copy link
Copy Markdown
Collaborator Author

/ok to test 815743a

@peterdsharpe
peterdsharpe marked this pull request as ready for review July 1, 2026 20:45
@greptile-apps

greptile-apps Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR standardizes tensor-valued gradient output across all PhysicsNeMo Mesh calculus paths (point/cell LSQ, intrinsic LSQ, and DEC) to the derivative-first layout (entity, n_spatial_dims, *value_shape), matching the documented API. It is an intentional breaking change for stored tensor gradients from earlier LSQ releases; scalar gradients are unaffected.

  • Removes _to_mesh_gradient_layout from _lsq_reconstruction.py and the corresponding permute from _lsq_intrinsic.py, and corrects a pre-existing weighting broadcast bug in the intrinsic path for fields with 2+ trailing value dimensions.
  • Updates _curl_from_jacobian index notation and _sharp_flat.sharp to handle arbitrarily shaped tensor 1-forms in the new layout, with a new regression test suite that uses a non-symmetric Jacobian to prevent false positives from transpositions.

Important Files Changed

Filename Overview
physicsnemo/mesh/calculus/_lsq_reconstruction.py Removes the _to_mesh_gradient_layout helper that permuted the derivative axis to last, directly returning the functional API output which already uses derivative-first layout.
physicsnemo/mesh/calculus/_lsq_intrinsic.py Fixes weighting broadcast for tensor fields with 2+ trailing dims, and removes the permute that moved the derivative axis to last; output is now consistently (entity, n_spatial_dims, *value_shape).
physicsnemo/mesh/calculus/_sharp_flat.py Generalizes the contrib_spatial_expanded view from a single hard-coded trailing singleton to edge_1form.ndim - 1 trailing singletons, supporting arbitrary tensor-valued 1-forms in derivative-first layout.
physicsnemo/mesh/calculus/curl.py Transposes index access in _curl_from_jacobian to match new jacobian[i, k, j] = ∂v_j/∂x_k layout; curl values are mathematically unchanged and the test confirms correctness.
physicsnemo/mesh/calculus/divergence.py Documentation-only update; the torch.einsum("...ii", jacobian) trace is invariant to transposition of the last two axes, so divergence values are unchanged.
physicsnemo/mesh/mesh.py Adds a clarifying docstring line about the derivative-first Jacobian convention for vector fields; no logic change.
test/mesh/calculus/test_gradient_layout.py New regression test suite covering point/cell, intrinsic/extrinsic, LSQ/DEC paths for scalar and tensor fields, plus divergence, curl, and stored derivative data. Uses a deliberately non-symmetric Jacobian to detect axis transposition.
CHANGELOG.md Documents the breaking change for stored tensor gradients and provides the migration path (legacy_gradient.movedim(-1, 1)).

Reviews (1): Last reviewed commit: "fix(mesh): standardize calculus gradient..." | Re-trigger Greptile

@peterdsharpe

Copy link
Copy Markdown
Collaborator Author

/ok to test ae5e985

@loliverhennigh loliverhennigh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@peterdsharpe
peterdsharpe added this pull request to the merge queue Jul 9, 2026
Merged via the queue into NVIDIA:main with commit cae63fe Jul 9, 2026
6 checks passed
@peterdsharpe
peterdsharpe deleted the codex/mesh-calculus-gradient-layout branch July 9, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants