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

Patched docs for torch_compile_tutorial #2936

Merged
merged 9 commits into from
Aug 30, 2024

Conversation

ignaciobartol
Copy link
Contributor

@ignaciobartol ignaciobartol commented Jun 16, 2024

Fixes #2858

Description

Based on the discussion on #2858 I added a few examples on how to use torch.compile in nested modules and functions. Also, I added a few lines for "Best practices" to debug if there is a compilation problem and also how to disable a function from being compiled (along with its children functions) using torch.compiler.disable.

Let me know if anything can be improved in the tutorial and I will work on it. As a side note, from this issue in pytorch, I am not sure if torch.compiler.disable is working properly, but I used as specified as in the docs here

Checklist

  • The issue that is being fixed is referred in the description (see above "Fixes #ISSUE_NUMBER")
  • Only one issue is addressed in this pull request
  • Labels from the issue that this PR is fixing are added to this pull request
  • No unnecessary issues are included into this pull request.

cc @williamwen42 @msaroufim @sekyondaMeta @svekars @kit1980 @brycebortree

Copy link

pytorch-bot bot commented Jun 16, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/tutorials/2936

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit f36b152 with merge base e07d43b (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Copy link
Member

@williamwen42 williamwen42 left a comment

Choose a reason for hiding this comment

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

I think this doc update would be better placed in the troubleshooting tutorial (@mlazos is working on this and will have an update soon).

intermediate_source/torch_compile_tutorial.py Show resolved Hide resolved
intermediate_source/torch_compile_tutorial.py Show resolved Hide resolved
#
# This includes:
#
# - **Nested function calls:** All functions called within the decorated or compiled function will also be compiled.
Copy link
Member

Choose a reason for hiding this comment

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

There isn't really too much of a difference in the nested call behavior between torch.compile'd functions and torch.compile'd modules, so I don't think we need to highlight them as distinct cases.

intermediate_source/torch_compile_tutorial.py Show resolved Hide resolved
#
# Behavior of ``torch.compile`` with Nested Modules and Function Calls
#
# When you use ``torch.compile``, the compiler will try to recursively inline
Copy link
Member

Choose a reason for hiding this comment

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

Don't have to mention inlining - going over how dynamo inlines is a little more involved.

# Behavior of ``torch.compile`` with Nested Modules and Function Calls
#
# When you use ``torch.compile``, the compiler will try to recursively inline
# and compile every function call inside the target function or module.
Copy link
Member

Choose a reason for hiding this comment

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

"every function call" is not exactly right - there is a skiplist of various functions (e.g. builtins, some functions in the torch.* namespace).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, so I will modify the wording adding your comment to something like "compile every function call inside the target function or module that is not in a skiplist (e.g. builtins, some functions in the torch.* namespace)."

@ignaciobartol
Copy link
Contributor Author

I think this doc update would be better placed in the troubleshooting tutorial (@mlazos is working on this and will have an update soon).

Thanks @williamwen42 for the review and the great feedback! I will work on them. Once I have an update on which doc to contribute I will create a new commit.

@ignaciobartol
Copy link
Contributor Author

@pytorchbot rebase

Copy link

pytorch-bot bot commented Jun 24, 2024

You don't have permissions to rebase this PR since you are a first time contributor. If you think this is a mistake, please contact PyTorch Dev Infra.

@ignaciobartol ignaciobartol force-pushed the torch-compile branch 2 times, most recently from 8aec180 to 204f9fc Compare June 24, 2024 12:33
Copy link
Member

@williamwen42 williamwen42 left a comment

Choose a reason for hiding this comment

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

Content looks good, but can you fix any formatting issues that you see in the doc preview?

Also waiting for response from @mlazos to see where this change should go

@mlazos
Copy link
Contributor

mlazos commented Jun 25, 2024

Thanks @ignaciobartol, this looks like a good tutorial! I think we should add a section called "Best Practices for Onboarding a New Model" to https://pytorch.org/docs/stable/torch.compiler_troubleshooting.html and add your best practices section to that. You can also link to it from your tutorial as well. The rest of the troubleshooting doc is a bit outdated and I'll be revamping that next week.

@ignaciobartol
Copy link
Contributor Author

Content looks good, but can you fix any formatting issues that you see in the doc preview?

Also waiting for response from @mlazos to see where this change should go

Thanks! I am not sure what I am seeing in my build of the docs differs from what is on the preview (See screenshot below). Any ideas of why this may be happening?

torch_compile_build

intermediate_source/torch_compile_tutorial.py Outdated Show resolved Hide resolved
intermediate_source/torch_compile_tutorial.py Outdated Show resolved Hide resolved
intermediate_source/torch_compile_tutorial.py Outdated Show resolved Hide resolved
intermediate_source/torch_compile_tutorial.py Outdated Show resolved Hide resolved
intermediate_source/torch_compile_tutorial.py Outdated Show resolved Hide resolved
@svekars
Copy link
Contributor

svekars commented Jun 25, 2024

From publishing perspective the PR looks good to me. @mlazos - did you mean that this should be moved to torch.compiler troubleshooting instead of being here?

@ignaciobartol
Copy link
Contributor Author

From publishing perspective the PR looks good to me. @mlazos - did you mean that this should be moved to torch.compiler troubleshooting instead of being here?

Thanks @svekars for fixing the formatting errors. Once we have more information about this I can ago ahead and apply those changes

@mlazos
Copy link
Contributor

mlazos commented Aug 20, 2024

From publishing perspective the PR looks good to me. @mlazos - did you mean that this should be moved to torch.compiler troubleshooting instead of being here?

Correct, it was just a suggestion, I'm okay either way.

@svekars svekars merged commit 748e52b into pytorch:main Aug 30, 2024
17 checks passed
c-p-i-o pushed a commit that referenced this pull request Sep 6, 2024
* Patched docs for torch_compile_tutorial

---------

Co-authored-by: Svetlana Karslioglu <svekars@meta.com>
c-p-i-o pushed a commit that referenced this pull request Sep 6, 2024
* Patched docs for torch_compile_tutorial

---------

Co-authored-by: Svetlana Karslioglu <svekars@meta.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better specify torch.compile behaviour on nested function/module
5 participants