Skip to content

Conversation

@jleinonen
Copy link
Collaborator

PhysicsNeMo Pull Request

On request by @CharlelieLrt, opening PR corresponding to #1149 but against the main branch.

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.

jleinonen and others added 30 commits October 3, 2025 06:35
style guide edits
Signed-off-by: Charlelie Laurent <[email protected]>
github.com:jleinonen/modulus into interp-model-example
Merge changes from CharlelieLrt
CharlelieLrt and others added 13 commits October 30, 2025 16:32
* Remove pyevtk

* update dependency

* update dimensions

* ci issues
@jleinonen
Copy link
Collaborator Author

/blossom-ci

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 24, 2025

Greptile Overview

Greptile Summary

Adds a new temporal interpolation example for weather forecasting using ModAFNO, including training scripts, datapipes, utilities, and comprehensive documentation. Also includes version bump to 1.3.0, dependency updates (einops, requests), and unrelated changes to Gray-Scott example (pyevtk → pyvista migration).

Key additions:

  • Complete temporal interpolation training pipeline with ModAFNO model
  • Custom InterpClimateDatapipe for loading interpolation training sequences
  • Latitude-weighted geometric L2 loss for weather data
  • Training and validation scripts with distributed training support
  • Configuration files for production and test runs
  • Comprehensive README with setup and usage instructions

Issues found:

  • Missing datetime import in validate.py:268 will cause runtime error when running validation script

Important Files Changed

File Analysis

Filename Score Overview
examples/weather/temporal_interpolation/validate.py 3/5 validation script with missing datetime import on line 268
examples/weather/temporal_interpolation/train.py 5/5 main training script with well-structured datapipe, model, and trainer setup
examples/weather/temporal_interpolation/datapipe/climate_interp.py 5/5 custom datapipe for interpolation training extending ClimateDatapipe
examples/weather/temporal_interpolation/utils/trainer.py 5/5 training loop with checkpoint management, validation, and wandb logging

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.

20 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

},
attrs={
"description": "Histogram of squared errors for temporal interpolation",
"created": datetime.now().isoformat(),
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax: missing import for datetime

Suggested change
"created": datetime.now().isoformat(),
"created": datetime.datetime.now().isoformat(),

@jleinonen
Copy link
Collaborator Author

/blossom-ci

Copy link
Collaborator

@coreyjadams coreyjadams left a comment

Choose a reason for hiding this comment

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

Hi @jleinonen - I haven't reviewed the PR, really, I'll let others get into the examples.

This PR has several changes that were fixed for the release that aren't here. I am not sure why, we'll track it down. You will likely have to rebase your PR off of that and undo some of these miscellaneous changes, please.

In the meantime, we shouldn't change the version number in this PR.

from .models.module import Module

__version__ = "1.3.0a0"
__version__ = "1.3.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't be changing the software version this commit.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ktangsali are you code owner on physicsnemo/__init__.py? If not, we should ensure that :)

from physicsnemo.utils.version_check import check_min_version

WARP_AVAILABLE = check_min_version("warp", "0.6.0")
WARP_AVAILABLE = check_min_version("warp", "0.6.0", hard_fail=False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was fixed in QA for release ...

Comment on lines +16 to +21
"einops>=0.8.0",
"fsspec>=2023.1.0",
"numpy>=1.22.4",
"onnx>=1.14.0",
"packaging>=24.2",
"requests>=2.32.2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

These were fixed in QA for release ...

"stl_faces": faces.astype(np.int32).flatten(),
"stl_centers": centroids.astype(np.float32),
"stl_areas": areas.astype(np.float32),
"air_density": np.float32(1.225), # Standard air density
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was addressed during QA for release ...

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.

5 participants