Skip to content

Mesh: normalize geometry robustly across dtypes and scales - #1883

Draft
peterdsharpe wants to merge 1 commit into
NVIDIA:mainfrom
peterdsharpe:pr/mesh-fp16-zero-normals
Draft

Mesh: normalize geometry robustly across dtypes and scales#1883
peterdsharpe wants to merge 1 commit into
NVIDIA:mainfrom
peterdsharpe:pr/mesh-fp16-zero-normals

Conversation

@peterdsharpe

@peterdsharpe peterdsharpe commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

PhysicsNeMo Pull Request

Description

torch.nn.functional.normalize(eps=1e-12) is not scale- or dtype-safe for mesh normals: the floor rounds to zero in float16 and turns zero vectors into NaN, norm formation can overflow and turn valid normals into zero, and legitimate small float32 or float64 vectors become non-unit normals.

This PR:

  • Adds safe_normalize, which first scales each vector by its largest absolute component so its norm cannot overflow or underflow.
  • Preserves exactly-zero vectors explicitly, retaining the documented zero normal for degenerate cells and points without incident cells.
  • Applies the helper to cell, point, cluster, and transformed normal paths.
  • Replaces one boolean-mask centroid assignment with equivalent torch.where logic to avoid a device synchronization.

The extra scaling pass makes the normalization kernel roughly 2-4x slower in isolation, but normals are cached per mesh and remain sub-millisecond at multi-million-cell scale; the tradeoff removes dependence on coordinate scale and dtype.

Verification

  • pytest test/mesh: 2370 passed, 701 skipped.
  • New tests fail against both the original F.normalize implementation and a zero-mask-only implementation that omits rescaling.
  • Coverage spans float16, bfloat16, float32, and float64, including zero, underflowing, and overflowing vectors.
  • ruff check, ruff format --check, and doctests pass.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • The CHANGELOG.md is up to date with these changes.
  • An issue is linked to this pull request.
  • Model implementation standards are not applicable; no model code is changed.

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 the work or an indication that the PR will be accepted or 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.

The hardcoded eps=1e-12 clamp in torch.nn.functional.normalize is not safe
across the dtypes and length scales mesh geometry uses. In float16 the floor
itself rounds to zero, so degenerate cells divide 0/0 and return NaN. Forming
the norm squares each component and overflows to inf for large cells, so a
well-conditioned normal silently becomes zero. And because the floor is
absolute rather than dtype-relative, a genuine norm below it is substituted,
so float32 and float64 alike return non-unit normals once feature size falls
below roughly 1e-6.

Add safe_normalize, which divides each vector by its own largest absolute
component before taking the norm and preserves exactly-zero vectors, and route
cell, point, cluster, and transformed normals through it. The rescaled norm is
bounded in [1, sqrt(n)], so it can neither overflow nor underflow and no
epsilon is needed.

Also replaces a boolean-mask centroid assignment in partition_cells with
torch.where, which is bit-identical and drops a nonzero device sync.
@copy-pr-bot

copy-pr-bot Bot commented Jul 28, 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 a659d7290111. An approval covers every file listed for that owner; one owner is sufficient for shared files.

@peterdsharpe — 9 file(s)
  • physicsnemo/mesh/geometry/_cell_normals.py
  • physicsnemo/mesh/mesh.py
  • physicsnemo/mesh/remeshing/_partition.py
  • physicsnemo/mesh/transformations/geometric.py
  • physicsnemo/mesh/utilities/_tolerances.py
  • test/mesh/geometry/test_cell_normals.py
  • test/mesh/mesh/test_point_normals_weighting.py
  • test/mesh/remeshing/test_partition.py
  • test/mesh/utilities/test_tolerances.py

No CODEOWNER

  • CHANGELOG.md

Comment /codeowners-info to refresh.

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.

1 participant