Skip to content

Mesh: preserve complex values during aggregation - #1879

Draft
peterdsharpe wants to merge 1 commit into
NVIDIA:mainfrom
peterdsharpe:pr/mesh-scatter-complex
Draft

Mesh: preserve complex values during aggregation#1879
peterdsharpe wants to merge 1 commit into
NVIDIA:mainfrom
peterdsharpe:pr/mesh-scatter-complex

Conversation

@peterdsharpe

@peterdsharpe peterdsharpe commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

PhysicsNeMo Pull Request

Description

Mesh mean and facet aggregation treated complex tensors as non-floating and cast them to float64, silently discarding imaginary values in public operations such as cell_data_to_point_data, get_facet_mesh, and duplicate-point merging. Complex weights supplied to sums could lose their imaginary contribution for the same reason.

This PR:

  • Keeps complex fields in their complex dtype during scatter and facet aggregation.
  • Uses real-valued weight sums for weighted means, where the divisor must be clamped, and reports complex mean weights with a clear TypeError.
  • Promotes weighted sums to the common complex dtype so both value and weight components are preserved.
  • Adds regression coverage for both facet data sources, weighted and unweighted aggregation, multidimensional fields, gradients, and CUDA.

Verification

  • pytest test/mesh: 2354 passed, 703 skipped.
  • 168 real-valued dtype/shape/aggregation configurations remain bit-identical to main, including gradients.
  • ruff check and ruff format --check pass for all changed files.

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.

Complex tensors are not "floating point" by torch's definition, so
scatter_aggregate and the point-to-facet vertex average promoted them to
float64 like an integer field, silently discarding the imaginary part.
This corrupted Mesh.cell_data_to_point_data, both data_source settings of
Mesh.get_facet_mesh, and repair.merge_duplicate_points.

A "mean" still requires real weights, since its divisor is clamped away
from zero and clamp rejects complex dtypes. A "sum" has no divisor, so it
accepts complex weights and promotes the accumulator to the common dtype
of the values and the weights.
@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 a88d075ca77c. An approval covers every file listed for that owner; one owner is sufficient for shared files.

@peterdsharpe — 5 file(s)
  • physicsnemo/mesh/boundaries/_facet_extraction.py
  • physicsnemo/mesh/utilities/_scatter_ops.py
  • test/mesh/boundaries/test_facet_extraction.py
  • test/mesh/mesh/test_data_conversion.py
  • test/mesh/utilities/test_scatter_ops.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