[dynamo] support inactive context managers across graph breaks #125203
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/125203
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 43ecd97 with merge base faee0e5 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
williamwen42
added a commit
that referenced
this pull request
Apr 30, 2024
ghstack-source-id: 48493ac520123ea1a0f98eff97f974bf90fa42b9 Pull Request resolved: #125203
williamwen42
added
topic: bug fixes
jansel
approved these changes
Apr 30, 2024
…eaks" Fix #124900. When we reconstruct `ContextWrappingVariables`s, we only reconstruct the context class, not the object. Normally, contexts are active (via `with ctx:`) and we initialize the context object in the resume function. But for the case of inactive contexts (contexts declared ahead of time before the `with` block), we do not reconstruct them properly in the optimized bytecode or resume function. So this PR adds initialization for inactive contexts in the resume function. cc ezyang msaroufim bdhirsh anijain2305 chauhang voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng [ghstack-poisoned]
williamwen42
added a commit
that referenced
this pull request
Apr 30, 2024
ghstack-source-id: 12c8724d98f5476c9777e652ba52181ee3c2444d Pull Request resolved: #125203
…eaks" Fix #124900. When we reconstruct `ContextWrappingVariables`s, we only reconstruct the context class, not the object. Normally, contexts are active (via `with ctx:`) and we initialize the context object in the resume function. But for the case of inactive contexts (contexts declared ahead of time before the `with` block), we do not reconstruct them properly in the optimized bytecode or resume function. So this PR adds initialization for inactive contexts in the resume function. cc ezyang msaroufim bdhirsh anijain2305 chauhang voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng [ghstack-poisoned]
williamwen42
added a commit
that referenced
this pull request
Apr 30, 2024
ghstack-source-id: 75077fa5e70b9c03483ce07cdfa1d6e1785bcf32 Pull Request resolved: #125203
|
@pytorchbot merge |
Merge startedYour 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 |
andoorve
pushed a commit
to andoorve/pytorch
that referenced
this pull request
May 1, 2024
…ch#125203) Fix pytorch#124900. When we reconstruct `ContextWrappingVariables`s, we only reconstruct the context class, not the object. Normally, contexts are active (via `with ctx:`) and we initialize the context object in the resume function. But for the case of inactive contexts (contexts declared ahead of time before the `with` block), we do not reconstruct them properly in the optimized bytecode or resume function. So this PR adds initialization for inactive contexts in the resume function. Pull Request resolved: pytorch#125203 Approved by: https://github.com/jansel
petrex
pushed a commit
to petrex/pytorch
that referenced
this pull request
May 3, 2024
…ch#125203) Fix pytorch#124900. When we reconstruct `ContextWrappingVariables`s, we only reconstruct the context class, not the object. Normally, contexts are active (via `with ctx:`) and we initialize the context object in the resume function. But for the case of inactive contexts (contexts declared ahead of time before the `with` block), we do not reconstruct them properly in the optimized bytecode or resume function. So this PR adds initialization for inactive contexts in the resume function. Pull Request resolved: pytorch#125203 Approved by: https://github.com/jansel
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack (oldest at bottom):
Fix #124900.
When we reconstruct
ContextWrappingVariabless, we only reconstruct the context class, not the object. Normally, contexts are active (viawith ctx:) and we initialize the context object in the resume function. But for the case of inactive contexts (contexts declared ahead of time before thewithblock), we do not reconstruct them properly in the optimized bytecode or resume function. So this PR adds initialization for inactive contexts in the resume function.cc @ezyang @msaroufim @bdhirsh @anijain2305 @chauhang @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng