Skip to content

Conversation

rtimpe
Copy link
Collaborator

@rtimpe rtimpe commented Aug 28, 2025

Stack from ghstack (oldest at bottom):

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo. This change defers the Unsupported exception
until the build_class builtin is actually called, which allows a graph break
to be inserted.

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

[ghstack-poisoned]
rtimpe added a commit that referenced this pull request Aug 28, 2025
Copy link

pytorch-bot bot commented Aug 28, 2025

🔗 Helpful Links

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

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

✅ No Failures

As of commit 7b22c90 with merge base 86d34a4 (image):
💚 Looks good so far! There are no failures yet. 💚

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

[ghstack-poisoned]
rtimpe added a commit that referenced this pull request Aug 28, 2025
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test (test_small_stability, which was deleted) was already passing before this PR. I'm not sure why

[ghstack-poisoned]
rtimpe added a commit that referenced this pull request Aug 28, 2025
[ghstack-poisoned]
rtimpe added a commit that referenced this pull request Aug 28, 2025
@pytorch-bot pytorch-bot bot added the release notes: fx release notes category label Aug 28, 2025
[ghstack-poisoned]
rtimpe added a commit that referenced this pull request Aug 29, 2025
test/test_fx.py Outdated
print(x)


@skipIfTorchDynamo("Not suitable for dynamo")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Some of these are failing during setup, but many define custom classes, so I think it's just easier to skip them all. If there's a desire to run these tests under dynamo, I can look at them more carefully

Copy link
Collaborator

Choose a reason for hiding this comment

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

Put these failures on test/dynamo_expected_failures

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Note that two of the tests had to go in dynamo_skips though (one was passing despite being expected to fail and the other was passing on python 3.10 but not 3.13)

@guilhermeleobas
Copy link
Collaborator

@williamwen42, can you review this one? Rob implemented the LOAD_BUILD_CLASS opcode, and we're now graph breaking when builtins.build_class() actually happens. This should avoid the frame being skipped because of set_fullgraph(False) region.

@rtimpe, we need to make sure the CPython tests are actually being traced by Dynamo and not skipped.

@rtimpe rtimpe marked this pull request as ready for review August 29, 2025 19:00
[ghstack-poisoned]
rtimpe added a commit that referenced this pull request Sep 3, 2025
@guilhermeleobas
Copy link
Collaborator

@rtimpe can you rebase this PR? You can do this with @pytorchbot rebase command

[ghstack-poisoned]
rtimpe added a commit that referenced this pull request Sep 4, 2025
Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.  Fixes #161562

ghstack-source-id: f18a913
Pull-Request: #161670
rtimpe added a commit that referenced this pull request Sep 4, 2025
Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.  Fixes #161562

ghstack-source-id: f18a913
Pull-Request: #161670
[ghstack-poisoned]
rtimpe added a commit that referenced this pull request Sep 4, 2025
Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.  Fixes #161562

ghstack-source-id: 95ad2ab
Pull-Request: #161670
[ghstack-poisoned]
[ghstack-poisoned]
rtimpe added a commit that referenced this pull request Sep 10, 2025
Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.

ghstack-source-id: fa23eef
Pull-Request: #161670
@rtimpe
Copy link
Collaborator Author

rtimpe commented Sep 10, 2025

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

markc-614 pushed a commit to markc-614/pytorch that referenced this pull request Sep 17, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.  Fixes pytorch#161562

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
markc-614 pushed a commit to markc-614/pytorch that referenced this pull request Sep 17, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
markc-614 pushed a commit to markc-614/pytorch that referenced this pull request Sep 17, 2025
…ion (pytorch#161670)"

This reverts commit e254548.

Reverted pytorch#161670 on behalf of https://github.com/huydhn due to Sorry for reverting your change but it is failing a trunk test ([comment](pytorch#161670 (comment)))
markc-614 pushed a commit to markc-614/pytorch that referenced this pull request Sep 17, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
mansiag05 pushed a commit to mansiag05/pytorch that referenced this pull request Sep 22, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.  Fixes pytorch#161562

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
mansiag05 pushed a commit to mansiag05/pytorch that referenced this pull request Sep 22, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
mansiag05 pushed a commit to mansiag05/pytorch that referenced this pull request Sep 22, 2025
…ion (pytorch#161670)"

This reverts commit e254548.

Reverted pytorch#161670 on behalf of https://github.com/huydhn due to Sorry for reverting your change but it is failing a trunk test ([comment](pytorch#161670 (comment)))
mansiag05 pushed a commit to mansiag05/pytorch that referenced this pull request Sep 22, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
cleonard530 pushed a commit to cleonard530/pytorch that referenced this pull request Sep 22, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.  Fixes pytorch#161562

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
cleonard530 pushed a commit to cleonard530/pytorch that referenced this pull request Sep 22, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
cleonard530 pushed a commit to cleonard530/pytorch that referenced this pull request Sep 22, 2025
…ion (pytorch#161670)"

This reverts commit e254548.

Reverted pytorch#161670 on behalf of https://github.com/huydhn due to Sorry for reverting your change but it is failing a trunk test ([comment](pytorch#161670 (comment)))
cleonard530 pushed a commit to cleonard530/pytorch that referenced this pull request Sep 22, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
dsashidh pushed a commit to dsashidh/pytorch that referenced this pull request Sep 26, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.  Fixes pytorch#161562

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
dsashidh pushed a commit to dsashidh/pytorch that referenced this pull request Sep 26, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
dsashidh pushed a commit to dsashidh/pytorch that referenced this pull request Sep 26, 2025
…ion (pytorch#161670)"

This reverts commit e254548.

Reverted pytorch#161670 on behalf of https://github.com/huydhn due to Sorry for reverting your change but it is failing a trunk test ([comment](pytorch#161670 (comment)))
dsashidh pushed a commit to dsashidh/pytorch that referenced this pull request Sep 26, 2025
…orch#161670)

Currently, user-defined classes inside of a compiled frame will cause the whole
frame to be skipped by dynamo.  This change defers the Unsupported exception
until the __build_class__ builtin is actually called, which allows a graph break
to be inserted.

Pull Request resolved: pytorch#161670
Approved by: https://github.com/williamwen42, https://github.com/guilhermeleobas
@github-actions github-actions bot deleted the gh/rtimpe/15/head branch October 11, 2025 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-no-td Do not run TD on this PR ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request keep-going Don't stop on first failure, keep running tests until the end Merged module: dynamo open source release notes: fx release notes category Reverted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

set_fullgraph(False) actually skips the frame if a graph break happens

7 participants