Skip to content

Crash inference bug fix#1423

Merged
mnabian merged 16 commits intoNVIDIA:mainfrom
mnabian:crash-inference-bug-fix
Feb 17, 2026
Merged

Crash inference bug fix#1423
mnabian merged 16 commits intoNVIDIA:mainfrom
mnabian:crash-inference-bug-fix

Conversation

@mnabian
Copy link
Collaborator

@mnabian mnabian commented Feb 17, 2026

PhysicsNeMo Pull Request

Description

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.

@mnabian mnabian self-assigned this Feb 17, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 17, 2026

Greptile Summary

This PR fixes a crash in the inference script by removing the tempfile.TemporaryDirectory + symlink approach (which was the source of the crash) and instead passing each run's directory path directly as data_dir to the dataset. It also adds multi-GPU inference documentation to the README and corrects a copyright year in d3plot_reader.py.

Key changes and issues found:

  • Core fix (inference.py): Replaces the fragile tempfile + os.symlink pattern with a direct data_dir=run_path pass. This is simpler and avoids the symlink-related crash. The README is updated to document the new required directory layout (run_<ID>/ subfolders containing the run files).
  • VTP output directory naming mismatch (inference.py:186): vtp_frames_dir is built from the run folder name (e.g., output_run_0), but vtp_reader.py creates its frame output directory from the VTP file stem (e.g., output_Run0 for Run0.vtp). Per the README's documented layout, these names differ — causing the os.path.isdir check to always fail and silently skip all VTP export despite inference completing normally.
  • Stale comment (inference.py:158): The inline comment still references "tmpdir" even though the temporary directory mechanism was removed.
  • Copyright year fix (d3plot_reader.py): Copyright year corrected from 2026 to 2025 — no functional impact.

Important Files Changed

Filename Overview
examples/structural_mechanics/crash/inference.py Removes tempfile+symlink approach and passes run_path directly as data_dir; introduces a naming mismatch between vtp_frames_dir (based on run folder name) and the output dirs created by vtp_reader.py (based on VTP filename stem), which will cause VTP export to always be skipped when filenames differ from run folder names.
examples/structural_mechanics/crash/d3plot_reader.py Only changes copyright year from 2026 to 2025 — no functional changes.
examples/structural_mechanics/crash/README.md Adds inference directory structure documentation and multi-GPU inference instructions; the documented run_<ID>/Run<N>.vtp layout highlights the naming discrepancy that could cause VTP export failure.

Last reviewed commit: d9c121d

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.

3 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@mnabian
Copy link
Collaborator Author

mnabian commented Feb 17, 2026

/blossom-ci

@mnabian mnabian enabled auto-merge February 17, 2026 19:07
@mnabian mnabian requested a review from ktangsali February 17, 2026 19:59
Copy link
Collaborator

@ktangsali ktangsali left a comment

Choose a reason for hiding this comment

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

LGTM

@mnabian mnabian added this pull request to the merge queue Feb 17, 2026
Merged via the queue into NVIDIA:main with commit b41b242 Feb 17, 2026
4 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.

2 participants