Skip to content

Conversation

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 18, 2025

🔗 Helpful Links

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

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ No Failures

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

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

…n test/dynamo/test_ctx_manager.py"

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

[ghstack-poisoned]
Copy link
Contributor

@Lucaskabela Lucaskabela left a comment

Choose a reason for hiding this comment

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

LGTM with clear semantics here :)

…n test/dynamo/test_ctx_manager.py"

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

[ghstack-poisoned]
@williamwen42 williamwen42 changed the title [dynamo, nested graph breaks] enable nested_graph_breaks in test/dynamo/test_ctx_manager.py [dynamo, nested graph breaks] disable nested graph breaks in generators; enable nested_graph_breaks in test_ctx_manager.py and test_generator.py Oct 20, 2025
super().tearDown()
torch._dynamo.config.enable_trace_contextlib = self._prev
torch._dynamo.config.enable_trace_unittest = self._u_prev
torch._dynamo.config.nested_graph_breaks = False
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't you save the previous value? Otherwise, you may falsify a flag that was originally True.

Copy link
Member Author

Choose a reason for hiding this comment

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

I fixed this in #166013

…in generators; enable nested_graph_breaks in test_ctx_manager.py and test_generator.py"


Generators should not support nested graph breaks.

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

[ghstack-poisoned]
…in generators; enable nested_graph_breaks in test_ctx_manager.py and test_generator.py"


Generators should not support nested graph breaks.

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

[ghstack-poisoned]
…in generators; enable nested_graph_breaks in test_ctx_manager.py and test_generator.py"


Generators should not support nested graph breaks.

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

[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Starting merge as part of PR stack under #166016

pytorchmergebot pushed a commit that referenced this pull request Oct 28, 2025
…166016)

As discussed offline with @ydwu4, we should not allow nested graph breaks in HOPs.

Pull Request resolved: #166016
Approved by: https://github.com/Lucaskabela
ghstack dependencies: #166013, #166015, #165808, #165809
Khanaksahu pushed a commit to Khanaksahu/pytorch-fork that referenced this pull request Nov 17, 2025
…rs; enable nested_graph_breaks in test_ctx_manager.py and test_generator.py

ghstack-source-id: bb4bb3c
Pull Request resolved: pytorch/pytorch#165809
Khanaksahu pushed a commit to Khanaksahu/pytorch that referenced this pull request Nov 17, 2025
…mo/test_ctx_manager.py

ghstack-source-id: 1e78191
Pull Request resolved: pytorch/pytorch#165809
@github-actions github-actions bot deleted the gh/williamwen42/313/head branch November 29, 2025 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants