Skip to content

Sym upstream pr2 examples#1586

Merged
ktangsali merged 7 commits intoNVIDIA:mainfrom
ktangsali:sym-upstream-pr2-examples
Apr 27, 2026
Merged

Sym upstream pr2 examples#1586
ktangsali merged 7 commits intoNVIDIA:mainfrom
ktangsali:sym-upstream-pr2-examples

Conversation

@ktangsali
Copy link
Copy Markdown
Collaborator

@ktangsali ktangsali commented Apr 24, 2026

PhysicsNeMo Pull Request

Description

PR 2 of Sym upstream efforts. In this PR we port the examples to use the new Sym interface and drop the geometry specific imports from previous sym in support of PhysicsNeMo.Mesh + PyVista.

Depends on #1567 (PhysicsInformer upstreaming - merged)

Changes

  • Import updates: Replace physicsnemo.sym.hydra, physicsnemo.sym.distributed, and physicsnemo.sym.eq.spatial_grads imports with their PhysicsNeMo equivalents
  • Inline PDE definitions: Replace pre-built PDE classes (NavierStokes, Diffusion, IncompressibleNavierStokes) with inline SymPy definitions in each example
  • Key/Arch removal: Replace Key objects and Arch base class with plain strings and torch.nn.Module
  • Geometry replacement: Drop physicsnemo.sym.geometry imports in favor of physicsnemo.mesh primitives, sample_random_points_on_cells, and PyVista for STL loading; analytical SDF for axis-aligned box domains
  • Derivative functional: Replace FirstDeriv with mesh_lsq_gradient from physicsnemo.nn.functional.derivatives in DoMINO physics loss
  • Documentation: Update README, FAQ, example READMEs, and migration guide to reflect that PhysicsNeMo-Sym is archived and its functionality is now built-in

All changes have been verified by running the actual example (in case of LDC) or a minimal test to check the reproducibility of the behavior using the new code changes

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.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 24, 2026

Greptile Summary

This PR ports PhysicsNeMo examples from the archived physicsnemo-sym package to the new physicsnemo.sym module, replacing geometry primitives with physicsnemo.mesh + PyVista, dropping Key/Arch wrappers in favor of plain torch.nn.Module, and inlining PDE definitions with SymPy. Two issues need attention before merging:

  • ldc_pinns/train.py: mask_no_slip (y < height/2) and mask_top_wall (y >= height/2 - 1e-7) overlap for a thin strip of boundary points, causing conflicting boundary condition loss signals in the same training step.
  • domino_nim_finetuning/src/train.py: The eqn dict is now built with {c.outputs[0]: c for c in ns.make_computations()} instead of eqn.make_nodes(return_as_dict=True). If the key type or structure differs, downstream physics-loss lookups could silently break.

Important Files Changed

Filename Overview
examples/cfd/ldc_pinns/train.py Replaces GeometryDatapipe/Rectangle with physicsnemo.mesh; refactors boundary sampling and training loop — overlapping mask_no_slip/mask_top_wall creates conflicting BC gradients (P1).
examples/cfd/external_aerodynamics/domino_nim_finetuning/src/train.py Removes FirstDeriv/IncompressibleNavierStokes imports; defines PDE inline and switches to make_computations() — potential API mismatch vs old make_nodes(return_as_dict=True) dict structure (P1).
examples/cfd/darcy_physics_informed/darcy_physics_informed_deeponet.py Replaces Arch/Key-based MdlsSymWrapper with plain torch.nn.Module DeepONet; inlines Diffusion PDE — duplicates the class with darcy_physics_informed_fno.py.
examples/cfd/external_aerodynamics/domino/src/loss.py Removes first_deriv parameter; replaces manual neighbor-loop gradients with mesh_lsq_gradient; duplicates _build_csr_from_neighbors with the nim-finetuning file.
examples/cfd/datacenter/inference.py Replaces Box/Channel geometry primitives with pure-NumPy SDF helpers; sign convention (positive inside) preserved and matches downstream sdf > 0 mask check.
v2.0-MIGRATION-GUIDE.md Adds a new PhysicsNeMo-Sym to physicsnemo.sym migration section with a clear import-mapping table and inline PDE example.
examples/cfd/darcy_physics_informed/darcy_physics_informed_fno.py Drops physicsnemo.sym.eq.pdes.diffusion import; inlines Diffusion PDE — duplicates the class definition that also appears in darcy_physics_informed_deeponet.py.
examples/cfd/external_aerodynamics/domino/src/train.py Removes first_deriv parameter from validation_step and train_epoch; all call sites updated consistently.
examples/cfd/stokes_mgn/pi_fine_tuning.py Replaces Arch/Key-based MdlsSymDNN with plain torch.nn.Module FourierDNN; manual concat/split replaces concat_input/split_output — looks correct.
examples/cfd/external_aerodynamics/xaeronet/surface/preprocessor.py Replaces Tessellation.from_stl with PyVista/from_pyvista; area-weighted sampling via torch.multinomial matches old boundary sampling interface.
examples/cfd/vortex_shedding_mgn/inference_analysis/custom_primitives.py Rewrites Point2D as a plain class dropping all sym geometry dependencies; SDF and boundary sampling logic faithfully reproduced.
examples/cfd/datacenter/train_physics_informed.py Inlines NavierStokes PDE using SymPy; drops physicsnemo.sym.eq.pdes.navier_stokes import; minor docstring additions only.

Comments Outside Diff (1)

  1. examples/cfd/external_aerodynamics/domino_nim_finetuning/src/train.py, line 993-995 (link)

    P1 make_computations() API produces a different dict structure than make_nodes(return_as_dict=True)

    The original code called eqn.make_nodes(return_as_dict=True), which returned a dict keyed by equation name (e.g., "continuity", "momentum_x", …). The replacement {c.outputs[0]: c for c in ns.make_computations()} keys the dict by the first output symbol of each computation object instead. Any downstream code that looks up eqn["continuity"] or similar string keys will receive a KeyError or silently use the wrong computation, breaking the physics-informed training.

    Please verify that make_computations() is the correct new API and that all eqn consumers use the new key type, or use the equivalent new API (e.g., ns.make_nodes(return_as_dict=True)) if it still exists in physicsnemo.sym.

Reviews (1): Last reviewed commit: "update documentation references of Sym" | Re-trigger Greptile

Comment thread examples/cfd/ldc_pinns/train.py Outdated
Comment thread examples/cfd/darcy_physics_informed/darcy_physics_informed_fno.py Outdated
Comment thread examples/cfd/external_aerodynamics/domino/src/loss.py
Comment thread examples/cfd/datacenter/inference.py
Copy link
Copy Markdown
Collaborator

@loliverhennigh loliverhennigh left a comment

Choose a reason for hiding this comment

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

LGTM

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Apr 27, 2026

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.

@ktangsali
Copy link
Copy Markdown
Collaborator Author

/blossom-ci

Comment thread examples/cfd/external_aerodynamics/domino/src/loss.py
@ktangsali
Copy link
Copy Markdown
Collaborator Author

Blossom CI passing. Merging

@ktangsali ktangsali merged commit fc08854 into NVIDIA:main Apr 27, 2026
3 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.

4 participants