Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NVFuser fixes for 23.11 Modulus release #197

Merged
merged 12 commits into from
Oct 24, 2023
Merged

NVFuser fixes for 23.11 Modulus release #197

merged 12 commits into from
Oct 24, 2023
+319 −234

Conversation

ktangsali
Copy link
Collaborator

@ktangsali ktangsali commented Oct 18, 2023 •
edited
Loading

Modulus Pull Request

Description

A switch to the latest 23.10 PyTorch container will bring a change in NVFuser APIs. This PR tries to update the fused SiLu code to reflect that. It also moves the nvfuser imports so that it does not error out if not available. Addresses #60

The pytests will now pass by skipping the tests that touch fused silu code (if nvfuser is not available). If someone tries to import the module explicitly, it will print the following error message (ref: https://github.com/NVIDIA/modulus/actions/runs/6568159141/job/17842143792):

Error: Either nvfuser is not installed or the version is incompatible. Please retry after installing correct version of nvfuser. The new version of nvfuser should be available in PyTorch container version >= 23.10. https://docs.nvidia.com/deeplearning/frameworks/pytorch-release-notes/index.html

This PR is open for review but should not be merged until the 23.10 container becomes publicly available.

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

@ktangsali ktangsali changed the title NVFuser fixes NVFuser fixes for 23.11 Modulus release Oct 18, 2023
@ktangsali ktangsali added 3 - Ready for Review Ready for review by team 5 - DO NOT MERGE Hold off on merging; see PR for details labels Oct 18, 2023
@ktangsali ktangsali self-assigned this Oct 18, 2023
@ktangsali ktangsali linked an issue Oct 18, 2023 that may be closed by this pull request
Make nvfuser optional #60
Closed
@ktangsali
Copy link
Collaborator Author

/blossom-ci

@ktangsali
Copy link
Collaborator Author

/blossom-ci

@ktangsali ktangsali requested a review from mnabian October 20, 2023 22:06
akshaysubr
akshaysubr reviewed Oct 23, 2023
View reviewed changes
modulus/models/layers/fused_silu.py Outdated Show resolved Hide resolved
modulus/models/gnn_layers/mesh_graph_mlp.py Show resolved Hide resolved
test/models/graphcast/test_cugraphops.py Outdated Show resolved Hide resolved
test/models/graphcast/test_cugraphops.py Show resolved Hide resolved
modulus/models/layers/fused_silu.py Show resolved Hide resolved
@ktangsali
Copy link
Collaborator Author

/blossom-ci

@ktangsali ktangsali added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 5 - DO NOT MERGE Hold off on merging; see PR for details labels Oct 24, 2023
@ktangsali ktangsali merged commit 97f79e5 into NVIDIA:main Oct 24, 2023
@ktangsali ktangsali deleted the nv-fuser-fixes branch December 8, 2024 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnabian mnabian mnabian approved these changes

@akshaysubr akshaysubr akshaysubr approved these changes

Assignees

@ktangsali ktangsali

Labels
3 - Ready for Review Ready for review by team 5 - Ready to Merge Testing and reviews complete, ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make nvfuser optional
3 participants
@ktangsali @akshaysubr @mnabian