-
Notifications
You must be signed in to change notification settings - Fork 96
Complete torch.compile
executor
#140
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
Conversation
for more information, see https://pre-commit.ci
This comment was marked as outdated.
This comment was marked as outdated.
I suggest renaming the existing executor to "concat_inductor". This is what it does, uses Inductor to fuse concatenation and surrounding operations. |
f2e355b
to
75bf7ea
Compare
…ete-torch-compile
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
58077e9
to
ba75ad8
Compare
Are these known flakes on Windows? FAILED thunder/tests/test_grad.py::test_vjp_correctness_getitem_torch_cpu_float64 - AssertionError: Scalars are not close!
Expected 75.60970520045251 but got 81.13586235571096.
Absolute difference: 5.526157155258446 (up to 1e-05 allowed)
Relative difference: 0.07308793415617461 (up to 1.3e-06 allowed)
FAILED thunder/tests/test_grad.py::test_phantom_grad_vs_torch_consistency_getitem_torch_cpu_bfloat16 - AssertionError: Tensor-likes are not close!
Mismatched elements: 112 / 140 (80.0%)
Greatest absolute difference: 4.0 at index (0, 0, 0) (up to 1e-05 allowed)
Greatest relative difference: 2.0 at index (0, 0, 0) (up to 0.016 allowed)
FAILED thunder/tests/test_grad.py::test_phantom_grad_vs_torch_consistency_getitem_torch_cpu_float64 - AssertionError: Tensor-likes are not close!
Mismatched elements: 112 / 140 (80.0%)
Greatest absolute difference: 7.0 at index (0, 0, 0) (up to 1e-07 allowed)
Greatest relative difference: 3.5 at index (0, 0, 0) (up to 1e-07 allowed)
FAILED thunder/tests/test_grad.py::test_phantom_grad_vs_torch_consistency_getitem_torch_cpu_float32 - AssertionError: Tensor-likes are not close!
Mismatched elements: 112 / 140 (80.0%)
Greatest absolute difference: 5.0 at index (0, 0, 0) (up to 1e-05 allowed)
Greatest relative difference: 2.5 at index (0, 0, 0) (up to 1.3e-06 allowed) |
@carmocca I usually assume that consistency tests are flakes, yeah. Re-run, and it should go away. If it doesn't, then it wasn't a flake :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Awesome work. Thank you @carmocca @IvanYashchuk @apaz-cli
if "inductor_cat" in self.compile: | ||
from thunder.executors.torch_compile import torch_compile_cat_ex as torch_compile_ex | ||
|
||
executors.insert(0, torch_compile_ex) | ||
elif "inductor" in self.compile: | ||
from thunder.executors.torch_compile import torch_compile_ex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to highlight that our nightly scripts and reporting depend on the current naming convention to monitor performance history. There was no real need to modify these benchmark options names in this PR in a breaking way. In the future, I suggest we explore different alternatives for modifications that alter existing behavior before finalizing the merge. Additionally, after merging, it's important to communicate these changes through various channels, not limited to GitHub, to ensure everyone is informed.
What does this PR do?
Adds an instance of
TorchCompileExecutor
namedtorch_compile_Ex
that registers all ofpytorch_executor
's andsdpa_ex
's operators.The rename of the existing executor
torch_compile_executor
totorch_compile_cat_ex
breaks backwards compatibility.Playground script with litgpt:
Output:
Fixes https://github.com/Lightning-AI/lit-thunder-LEGACY/issues/2141
cc @Borda @apaz-cli