8000 [dynamo] Don't affect stack traces under TORCHDYNAMO_DISABLE by xmfan · Pull Request #148618 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

[dynamo] Don't affect stack traces under TORCHDYNAMO_DISABLE #148618

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
Open
wants to merge 8 commits into
base: gh/xmfan/193/base
Choose a base branch
from
Open

[dynamo] Don't affect stack traces under TORCHDYNAMO_DISABLE #148618

wants to merge 8 commits into from

Conversation

xmfan
Copy link
Member
@xmfan xmfan commented Mar 5, 2025 •
edited by pytorchmergebot
Loading

Stack from ghstack (oldest at bottom):

Follow-up to https://fb.workplace.com/groups/1286739428954016/permalink/1446477902980167/. We don't want to wrap eager code when TORCHDYNAMO_DISABLE/JustKnobs are flipped, because it was confusing model owners to still see their exception stack traces go through eval_frame.py.

8000

We already had it supported for torch.compile, this PR adds support for disable and run.

# err.py
import torch

@torch._dynamo.disable
def fn():
    raise Exception("hi")

fn()

# Before
> TORCHDYNAMO_DISABLE="1" python err.py
Traceback (most recent call last):
  File "/home/xmfan/core/a/pytorch/err.py", line 7, in <module>
    fn()
  File "/home/xmfan/core/a/pytorch/torch/_dynamo/eval_frame.py", line 828, in _fn
    return fn(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^
  File "/home/xmfan/core/a/pytorch/err.py", line 5, in fn
    raise Exception("hi")
Exception: hi

# After
> TORCHDYNAMO_DISABLE="1" python err.py
Traceback (most recent call last):
  File "/home/xmfan/core/a/pytorch/err.py", line 7, in <module>
    fn()
  File "/home/xmfan/core/a/pytorch/err.py", line 5, in fn
    raise Exception("hi")
Exception: hi

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames

@xmfan
Update
dbce49f 
[ghstack-poisoned]
@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented Mar 5, 2025 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

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

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

xmfan added a commit that referenced this pull request Mar 5, 2025
@xmfan
[dynamo] No eager code wrapping on TORCHDYNAMO_DISABLE
b374033 
ghstack-source-id: 69ad1ad
Pull Request resolved: #148618
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link
linux-foundation-easycla bot commented Mar 5, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@xmfan
Update
a292ffd 
[ghstack-poisoned]
xmfan added a commit that referenced this pull request Mar 5, 2025
@xmfan
[dynamo] No eager code wrapping on TORCHDYNAMO_DISABLE
663e1ea 
ghstack-source-id: 6d0a6ae
Pull Request resolved: #148618
@xmfan xmfan changed the title [dynamo] No eager code wrapping on TORCHDYNAMO_DISABLE [dynamo] Don't affect stack traces with TORCHDYNAMO_DISABLE Mar 5, 2025
@xmfan xmfan changed the title [dynamo] Don't affect stack traces with TORCHDYNAMO_DISABLE [dynamo] Don't affect stack traces under TORCHDYNAMO_DISABLE Mar 6, 2025
@xmfan xmfan marked this pull request as ready for review March 6, 2025 03:23
@xmfan
Copy link
Member Author
xmfan commented Mar 7, 2025

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@xmfan
Update
6a0eb8f 
[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Successfully rebased gh/xmfan/193/orig onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/148618)

pytorchmergebot pushed a commit that referenced this pull request Mar 7, 2025
@xmfan
[dynamo] No eager code wrapping on TORCHDYNAMO_DISABLE
090d5c8 
ghstack-source-id: 9e0cdfe
Pull Request resolved: #148618
@xmfan
Copy link
Member Author
xmfan commented Mar 8, 2025

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@xmfan
Update
59cf6e5 
[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Successfully rebased gh/xmfan/193/orig onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/148618)
8000

pytorchmergebot pushed a commit that referenced this pull request Mar 8, 2025
@xmfan
[dynamo] No eager code wrapping on TORCHDYNAMO_DISABLE
261c037 
ghstack-source-id: b93c2ff
Pull Request resolved: #148618
@xmfan
Update
106c7b6 
[ghstack-poisoned]
xmfan added a commit that referenced this pull request Mar 11, 2025
@xmfan
[dynamo] No eager code wrapping on TORCHDYNAMO_DISABLE
5073493 
ghstack-source-id: e4ecac2
Pull Request resolved: #148618
@xmfan
Update
e77179b 
[ghstack-poisoned]
xmfan added a commit that referenced this pull request Mar 12, 2025
@xmfan
[dynamo] No eager code wrapping on TORCHDYNAMO_DISABLE
45e1380 
ghstack-source-id: edb2a2f
Pull Request resolved: #148618
@xmfan
Update
fd183e3 
[ghstack-poisoned]
xmfan added a commit that referenced this pull request Mar 14, 2025
@xmfan
[dynamo] No eager code wrapping on TORCHDYNAMO_DISABLE
684a2d9 
ghstack-source-id: e0f560e
Pull Request resolved: #148618
@xmfan
Update
afd1af0 
[ghstack-poisoned]
xmfan added a commit that referenced this pull request Mar 14, 2025
@xmfan
[dynamo] No eager code wrapping on TORCHDYNAMO_DISABLE
391e1db 
ghstack-source-id: b01bd5f
Pull Request resolved: #148618
xmfan
xmfan commented Mar 14, 2025
View reviewed changes
torch/testing/_internal/optests/generate_tests.py
_is_inside_opcheck_mode.value = True
os.environ["TORCHDYNAMO_DISABLE"] = "1"
Copy link
Member Author

Choose a reason for hiding this comment

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

@williamwen42 @zou3519 with this PR, dynamically changing TORCHDYNAMO_DISABLE from inside the test caused some infinite recursion issues. I'm not exactly sure why, but if we are going to disable dynamo on all OpCheckMode tests any way, I changed it to use skipIfTorchDynamo

Copy link
Member Author

Choose a reason for hiding this comment

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

This PR will make the disable decorator no-op, but when we enter OpCheckMode in the generated tests, some frames have already been disabled/compiled under PYTORCH_TEST_WITH_DYNAMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a feature that opcheck is a no-op underneath PYTORCH_TEST_WITH_DYNAMO. It's not just these opcheck tests in PyTorch's test suite, we ran into a case with other tests (#111685 mentions some fbgemm tests)

zou3519
zou3519 requested changes Mar 17, 2025
View reviewed changes
Copy link
Contributor
@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

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

the TORCHDYNAMO_DISABLE is a feature, if we want to remove it we need to figure out how to apply the same logic to opcheck itself (instead of it being a decorator on the tests)

@github-actions GitHub Actions
Copy link
Contributor

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the Stale label May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zou3519 zou3519 zou3519 requested changes

@williamwen42 williamwen42 williamwen42 approved these changes

@anijain2305 anijain2305 Awaiting requested review from anijain2305

Assignees
No one assigned
Labels
ciflow/inductor module: dynamo release notes: dynamo Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@xmfan @pytorchmergebot @zou3519 @williamwen42
0