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.

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

Add nv_enable_options and nv_disable_options #1432

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

Priya2698
Copy link
Collaborator

@Priya2698 Priya2698 commented Nov 13, 2024

nvFuser version >= 0.2.23 will support setting NVFUSER_ENABLE/NVFUSER_DISABLE through the python frontend using _enable_options and _disable_options flags. This is to enable matmul/linear codegen for nvfuser through Thunder using flags nv_enable_options=["fuse_matmul"], nv_disable_options=["matmul_expr_eval"]
Issue: NVIDIA/Fuser#3022

nvFuser PR: NVIDIA/Fuser#3270

@Priya2698 Priya2698 marked this pull request as ready for review November 13, 2024 06:50
@mruberry
Copy link
Collaborator

Maybe @jjsjann123 or @kevinstephano would like to review this?

Copy link
Collaborator

@jjsjann123 jjsjann123 left a comment

Choose a reason for hiding this comment

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

nitpick on the test. Code change looks good to me otherwise.

thunder/executors/nvfuserex_impl.py Outdated Show resolved Hide resolved
thunder/tests/test_nvfuser.py Show resolved Hide resolved
@t-vi
Copy link
Collaborator

t-vi commented Nov 18, 2024

What would be sane defaults here?
How will user know they need to deviate?
So here we are adding two options that users need to know about and then they need to know the magic strings to be filled in...

@Priya2698
Copy link
Collaborator Author

Priya2698 commented Nov 18, 2024

What would be sane defaults here? How will user know they need to deviate?

By default, no options need to be used.

The primary objective of these options is nvfuser development.
We can enable matmul/linear for nvfuser using nv_enable_matmul/linear flags. Within nvfuser, we can further control the scheduler used for these operators (i.e. whether we use ATen or nvfuser codegen) through NVFUSER_ENABLE/DISABLE environment flags.

This PR coupled with NVIDIA/Fuser#3270 allows us to enable nvfuser codegen for matmul/linear through Thunder using these options. This is to primarily support direct experimentation on nvfuser's matmul scheduler from Thunder.

@mruberry mruberry enabled auto-merge (squash) November 21, 2024 16:20
@mruberry
Copy link
Collaborator

What would be sane defaults here? How will user know they need to deviate? So here we are adding two options that users need to know about and then they need to know the magic strings to be filled in...

These are good questions. I think it's OK for now because these are just developer-facing options to continue experimentation. Maybe we should review how options are presented more generally, to clarify what is for practitioners vs. internal developers? Maybe developer options should be better documented somewhere, too?

@Priya2698
Copy link
Collaborator Author

Priya2698 commented Nov 21, 2024

What would be sane defaults here? How will user know they need to deviate? So here we are adding two options that users need to know about and then they need to know the magic strings to be filled in...

These are good questions. I think it's OK for now because these are just developer-facing options to continue experimentation. Maybe we should review how options are presented more generally, to clarify what is for practitioners vs. internal developers?

We could rename the above options to _nv_enable/disable_options to hint these are not user-facing options?

Maybe developer options should be better documented somewhere, too?

I can add a wiki to the nvfuser page listing our options and/or add those to error messages in nvfuser.
Not sure how Thunder may want to display/document this info.

@mruberry
Copy link
Collaborator

What would be sane defaults here? How will user know they need to deviate? So here we are adding two options that users need to know about and then they need to know the magic strings to be filled in...

These are good questions. I think it's OK for now because these are just developer-facing options to continue experimentation. Maybe we should review how options are presented more generally, to clarify what is for practitioners vs. internal developers?

We could rename the above options to _nv_enable/disable_options to hint these are not user-facing options?

That's a cool idea! Or maybe "developer_nv_enable" or something? Let's not rename anything just yet, but a different naming scheme could make this clearer.

Maybe developer options should be better documented somewhere, too?

I can add a wiki to the nvfuser page listing our options and/or add those to error messages in nvfuser.

Also a great idea!

Again: nothing required now. Maybe the Thunder github repo should havea wiki, too

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