Skip to content

FlipBoundaryOrientation: reverse the winding with Mesh.with_cells - #2268

Merged
peterdsharpe merged 3 commits into
NVIDIA:2.3.0-rcfrom
peterdsharpe:pr/flip-boundary-with-cells
Nov 13, 2026
Merged

peterdsharpe merged 3 commits into
NVIDIA:2.3.0-rcfrom
peterdsharpe:pr/flip-boundary-with-cells

Conversation

@peterdsharpe

@peterdsharpe peterdsharpe commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

This PR targets 2.3.0-rc, which lacks #2266, so it also backports #2266. In this PR, mesh.py, measure.py and four mesh test files match main byte for byte, where #2266 was reviewed. CHANGELOG.md gains #2266's entry.

On 2.3.0-rc, FlipBoundaryOrientation, added in #2261, reverses a winding by building a new Mesh. There, Mesh.with_cells rescales the effective measures of reordered cells and can raise on zero-area ones. The backported #2266 makes it keep each measure, and its area, bit for bit. The transform now calls it.

A flipped mesh with effective measures now also keeps each cell's cached area bit for bit, outside tracing and autograd. Before, its areas were recomputed in the new vertex order, which in float32 differs from the area each measure came from. Normals named in normal_fields are still negated, and the normals cached for the old winding are still dropped.

This PR's own change removes 10 lines and adds 7 in transforms.py, plus one test. The test flips a sphere with effective measures and checks its measures and areas bit for bit, where recomputed areas differ. The datapipes transform tests and the unified aero recipe's tests pass.

Checklist

Dependencies

#2266, which this PR backports to 2.3.0-rc.

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.

…only reordered (NVIDIA#2266)

* Keep measures of reordered cells in Mesh.with_cells

A row that only reorders its own vertices, such as a winding flip, is the
same simplex: it now keeps its effective measure, cached area, and cached
centroid bitwise, instead of being rescaled by float32 roundoff in the area
ratio or raising on a zero-area cell with a nonzero measure. Rows whose
vertex set changes keep the area-ratio transfer.

Signed-off-by: Peter Sharpe <peterdsharpe@gmail.com>

* Accept unsigned replacement cells in Mesh.with_cells

The reordered-row check compared the raw replacement with the mesh's
int64 cells, and PyTorch cannot promote uint16, uint32 or uint64 with
int64. Compare result.cells, which the constructor has normalized.

Signed-off-by: Peter Sharpe <peterdsharpe@gmail.com>

* Make the dtype comment in Mesh.with_cells exact

Signed-off-by: Peter Sharpe <peterdsharpe@gmail.com>

* Document the caches Mesh.with_cells keeps, and widen its tests

Returns and Notes now say which caches the result holds without keep,
and that they are computed eagerly for all cells. _transfer_cell_measures
documents how unchanged indexes rows when parents is given, and says why
it caches the old area bits. New tests cover a compiled flip, an empty
mesh, keep=("cell", "areas"), and CUDA for the measure tests; the
centroid tests use a sentinel and the sliver test checks the cached area,
so each fails if its value is recomputed.

Signed-off-by: Peter Sharpe <peterdsharpe@gmail.com>

---------

Signed-off-by: Peter Sharpe <peterdsharpe@gmail.com>
Since NVIDIA#2266, Mesh.with_cells keeps the effective measure of each cell whose
vertices are only reordered, with the area it came from, so the transform
uses it instead of building a new Mesh. A flipped mesh with effective
measures now also keeps each cell's cached area bit for bit, where recomputed
float32 areas differ between the two vertex orders.

Signed-off-by: Peter Sharpe <peterdsharpe@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Nov 13, 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 057b878

@github-actions

Copy link
Copy Markdown
Contributor

CODEOWNERS review map

Current for commit 057b8780776f. An approval covers every file listed for that owner; one owner is sufficient for shared files.

⏳ @coreyjadams — 2 file(s)
  • physicsnemo/datapipes/transforms/mesh/transforms.py
  • test/datapipes/transforms/test_flip_boundary_orientation.py
⏳ @negin513 — 2 file(s)
  • physicsnemo/datapipes/transforms/mesh/transforms.py
  • test/datapipes/transforms/test_flip_boundary_orientation.py
⏳ @peterdsharpe — 1 file(s)
  • physicsnemo/datapipes/transforms/mesh/transforms.py

Comment /codeowners-info to refresh.

@peterdsharpe
peterdsharpe changed the base branch from main to 2.3.0-rc November 13, 2026 21:26
@peterdsharpe
peterdsharpe marked this pull request as ready for review November 13, 2026 21:26
@peterdsharpe

Copy link
Copy Markdown
Collaborator Author

/ok to test a9c8477

@greptile-apps

greptile-apps Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in Greptile

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

This PR preserves effective cell measures and their matching areas when cell vertices are reordered, and routes FlipBoundaryOrientation through that behavior.

  • Reordering a cell’s vertices keeps its effective measure and matching cached values.

No actionable issues found.

Reviews (1) · Last reviewed commit: "FlipBoundaryOrientation: reverse the win..." · Reviewed by Greptile

@negin513 negin513 left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The transforms.py change and its test look good: with_cells + with_data is simpler than rebuilding the Mesh, and the new test is non-vacuous (it asserts recomputed float32 areas differ).

@negin513 negin513 left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!! In fact Looks great to me. :-)

@peterdsharpe
peterdsharpe merged commit ac31d92 into NVIDIA:2.3.0-rc Nov 13, 2026
13 checks passed
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