Skip to content

Conversation

@coreyjadams
Copy link
Collaborator

@coreyjadams coreyjadams commented Nov 20, 2025

PhysicsNeMo Pull Request

This PR brings several updates:

  • The Transolver datapipe has been simplified and largely merged with DoMINO's. This is a great performance boost.
  • The Transolver example supports volumetric training.
  • The Transolver++ model is, optionally, supported.

Still minor details to wrap up:

  • Add plots and training results to the README for transolver.
  • Update the changelog.
  • Ensure the inference_on_zarr script works on volumetric data
  • Update or deprecate the inference on stl script, since we can evaluate R2, etc., right from zarr.

Description

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.

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.

…taset.

Surface dataloading is prototyped, not finished yet.
… data.

Applied some cleanup to make the datapipe similar to domino, which
is a step towards unification.
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 20, 2025

Greptile Overview

Greptile Summary

Overview

This PR introduces significant enhancements to the PhysicsNeMo transolver capabilities:

  • New Typhon Model: Geometry-Aware Physics Attention transformer extending Transolver with cross-attention to geometry and global context
  • Volumetric Training Support: Transolver now supports both surface and volumetric training modes
  • Unified Datapipe: Merged Transolver and DoMINO datapipes for improved performance and consistency
  • Transolver++ Features: Optional support for advanced features including gumbel softmax and learnable temperature

Critical Issue

Typhon Model Bug (physicsnemo/experimental/models/typhon/typhon.py:809-816): The embedding_states variable is referenced without being assigned when neither geometry nor global embeddings are provided. This will cause a runtime UnboundLocalError when the model is instantiated without optional context inputs.

Architecture Changes

The datapipe refactoring consolidates preprocessing logic and improves flexibility with configurable symmetries (translational invariance, scale invariance). The new CAEDataset enhancements support reading zarr attributes and more flexible volume key detection.

Checklist Status

Per the PR description, the following items remain incomplete:

  • CHANGELOG.md update pending
  • Training results/plots for README pending
  • Basic unit tests for Typhon pending (acknowledged as acceptable for experimental)

Important Files Changed

File Analysis

Filename Score Overview
physicsnemo/experimental/models/typhon/typhon.py 2/5 New Typhon model with critical bug: embedding_states undefined when no geometry/global embeddings provided
physicsnemo/datapipes/cae/transolver_datapipe.py 5/5 New unified datapipe merging Transolver and DoMINO pipelines for surface/volume training
physicsnemo/models/transolver/Physics_Attention.py 5/5 Enhanced with Transolver++ support (gumbel_softmax) and optional transformer-engine imports
examples/cfd/external_aerodynamics/transolver/src/train.py 5/5 Unified training script supporting both Transolver and Typhon with FP8/mixed precision
physicsnemo/datapipes/cae/cae_dataset.py 5/5 Enhanced to support zarr attributes and more flexible volume key detection

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

22 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +809 to +816
if len(global_context_input) > 0:
embedding_states = torch.cat(global_context_input, dim=-1)

# Project the inputs to the hidden dimension:
x = self.preprocess(local_embedding)

for block in self.blocks:
x = block(x, embedding_states)
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: embedding_states referenced before assignment when no geometry or global embeddings provided

Suggested change
if len(global_context_input) > 0:
embedding_states = torch.cat(global_context_input, dim=-1)
# Project the inputs to the hidden dimension:
x = self.preprocess(local_embedding)
for block in self.blocks:
x = block(x, embedding_states)
# Construct the embedding states:
if len(global_context_input) > 0:
embedding_states = torch.cat(global_context_input, dim=-1)
else:
embedding_states = None
# Project the inputs to the hidden dimension:
x = self.preprocess(local_embedding)
for block in self.blocks:
x = block(x, embedding_states)

Copy link
Collaborator

@RishikeshRanade RishikeshRanade left a comment

Choose a reason for hiding this comment

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

LGTM

inference workloads are different, so these aim to cover common scenarios as examples. -->

The validation dataset in Zarr format can be loaded, processed, and the L2
metrics summarized in `inference_on_zarr.py`. For surface data, this script will also
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add that return_mesh_neighbors should be set to true to run this?


metrics = {
"l2_pressure": torch.mean(l2[:, 0]),
"l2_pressure_surf": torch.mean(l2[:, 0]),
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the readme we need to mention that this part needs to be changed when extending to a different use case. The other way is to describe variables in config.yaml like domino and read them from there.



def pad_input_for_fp8(features: torch.Tensor, embeddings: torch.Tensor) -> torch.Tensor:
def pad_input_for_fp8(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do these need to be part of train.py? Can these functions be moved to utils or part of datapipe?

dataloader: Training data loader
sampler (torch.utils.data.Sampler): Sampler for distributed or sequential sampling.
model (torch.nn.Module): The neural network model to train.
epoch_len (int): Length of the epoch.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this number of epochs?

[2025-11-19 07:02:38,387][training][INFO] - Summary:
| Batch | Loss | L2 Pressure | L2 Shear X | L2 Shear Y | L2 Shear Z | Predicted Drag Coefficient | Pred Lift Coefficient | True Drag Coefficient | True Lift Coefficient | Elapsed (s) |
|---------|--------|---------------|--------------|--------------|--------------|------------------------------|-------------------------|-------------------------|-------------------------|---------------|
| Mean | 0.0311 | 0.0614 | 0.0921 | 0.108 | 0.1214 | 5.2949 | 1.9137 | 5.2962 | 1.9329 | 11.4647 |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this updated? These numbers look higher than what we discussed, right?

@coreyjadams coreyjadams changed the title Transolver volume + Typhon Model Transolver volume Nov 21, 2025
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