Skip to content

Mesh.with_cells: keep effective measures of cells whose vertices are only reordered - #2266

Merged
peterdsharpe merged 4 commits into
NVIDIA:mainfrom
peterdsharpe:pr/mesh-with-cells-measures
Nov 13, 2026
Merged

peterdsharpe merged 4 commits into
NVIDIA:mainfrom
peterdsharpe:pr/mesh-with-cells-measures

Conversation

@peterdsharpe

@peterdsharpe peterdsharpe commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

Mesh.with_cells rescales each effective measure by the new cell area over the old one. When a row only reorders its own vertices, as in a winding flip, the cell is the same simplex. Its measure should not change, but float32 areas differ between vertex orders.

import torch
from physicsnemo.mesh import Mesh
from physicsnemo.mesh.calculus.measure import cell_measures, set_cell_measures

mesh = Mesh(
    points=torch.tensor([[0.0, 0.0, 0.0], [1.0, 0.0, 0.0], [2.0, 0.0, 0.0]]),
    cells=torch.tensor([[0, 1, 2]]),
)
set_cell_measures(mesh, torch.tensor([1.5]))
flipped = mesh.with_cells(mesh.cells[:, [0, 2, 1]])
# main:    ValueError: Cannot transform nonzero effective measure on a zero-measure cell
# this PR: cell_measures(flipped) is tensor([1.5000])

A winding flip, with each measure set to twice its cell's area (1.5 for the zero-area cell):

Cells main This PR
10,000 random triangles 26% of the measures change, by up to 2.7e-6 relative unchanged, bit for bit
100,000 near-degenerate triangles measures scale by 0.022 to 26 unchanged, bit for bit
One zero-area cell with a nonzero measure ValueError unchanged

fix_orientation flips cells through with_cells, so it raised on such a cell too.

with_cells now marks each row whose sorted vertex indices equal the old row's. Those rows keep their effective measure and cached cell area bitwise, so a later transform's area ratio starts from the same pair. They also keep a cached centroid. Rows whose vertices change keep the area-ratio transfer and its error. The selection uses torch.where, adds no host sync, and writes no cache while tracing or when points require gradients.

New tests cover flips of subsampled meshes and slivers, flips and cyclic shifts of a zero-area cell, changed rows, cached centroids, unsigned and empty replacements, keep, gradients, torch.compile and fix_orientation.

Checklist

Dependencies

None.

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.

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>
@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.

@github-actions

Copy link
Copy Markdown
Contributor

CODEOWNERS review map

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

⏳ @peterdsharpe — 5 file(s)
  • physicsnemo/mesh/calculus/measure.py
  • physicsnemo/mesh/mesh.py
  • test/mesh/calculus/test_measure.py
  • test/mesh/repair/test_orientation.py
  • test/mesh/utilities/test_cache.py

No CODEOWNER

  • CHANGELOG.md

Comment /codeowners-info to refresh.

@peterdsharpe
peterdsharpe marked this pull request as ready for review November 13, 2026 17:14
@peterdsharpe
peterdsharpe requested a review from mnabian November 13, 2026 17:15
@greptile-apps

greptile-apps Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in Greptile

[Medium impact] The change affects mesh connectivity replacement and orientation repair.

Fix the unsigned-index comparison before merging.

Findings

  1. P1 Unsigned replacement cells fail ▶

Summary

This PR preserves effective measures when a cell’s vertices are reordered, including zero-area cells.

  • Reordered cell vertices keep the same measure and cached geometry.

Reviews (1) · Last reviewed commit: "Keep measures of reordered cells in Mesh..." · Reviewed by Greptile

Comment thread physicsnemo/mesh/mesh.py Resolved
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>
Signed-off-by: Peter Sharpe <peterdsharpe@gmail.com>

@mnabian mnabian 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. Left a few minor comments.

Comment thread physicsnemo/mesh/mesh.py Resolved
Comment thread physicsnemo/mesh/mesh.py Resolved
Comment thread physicsnemo/mesh/calculus/measure.py Resolved
Comment thread physicsnemo/mesh/calculus/measure.py Resolved
Comment thread test/mesh/calculus/test_measure.py Resolved
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>
@peterdsharpe

Copy link
Copy Markdown
Collaborator Author

/ok to test 2f72cd5

@peterdsharpe
peterdsharpe added this pull request to the merge queue Nov 13, 2026
Merged via the queue into NVIDIA:main with commit 335152b Nov 13, 2026
14 of 15 checks passed
peterdsharpe added a commit that referenced this pull request Nov 13, 2026
)

* Mesh.with_cells: keep effective measures of cells whose vertices are only reordered (#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>

* FlipBoundaryOrientation: reverse the winding with Mesh.with_cells

Since #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>

---------

Signed-off-by: Peter Sharpe <peterdsharpe@gmail.com>
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