Skip to content
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

[dynamo 3.11] changes to with contexts #94101

Closed
wants to merge 22 commits into from
Closed

[dynamo 3.11] changes to with contexts #94101

wants to merge 22 commits into from

Conversation

williamwen42
Copy link
Member

@williamwen42 williamwen42 commented Feb 4, 2023 •
edited
Loading

@williamwen42 williamwen42 mentioned this pull request Feb 4, 2023
Closed
@pytorch-bot
Copy link

pytorch-bot bot commented Feb 4, 2023 •
edited
Loading

🔗 Helpful Links

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

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

❌ 1 Failures

As of commit 2261640:

NEW FAILURES - The following jobs have failed:

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

This was referenced Feb 4, 2023
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@williamwen42 williamwen42 mentioned this pull request Feb 4, 2023
Closed
@williamwen42 williamwen42 added this to the Python 3.11 milestone Feb 4, 2023
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
1c0c068 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
7718111 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
0433dad 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42 williamwen42 mentioned this pull request Feb 7, 2023
Closed
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
ea84842 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
f16f6b7 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
7a8a6c5 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
773dd1e 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
albanD
albanD reviewed Feb 7, 2023
View reviewed changes
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

I was expecting to see some fix of the exception table alongside the new cleanup code?

torch/_dynamo/symbolic_convert.py
if (
isinstance(fn, WithExitFunctionVariable)
and len(self.block_stack) > 0
and id(fn) == self.block_stack[-1].id
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could there be more than one associated with this function?

torch/_dynamo/symbolic_convert.py
@@ -897,7 +900,7 @@ def SETUP_WITH(self, inst):
self.push(ctx.enter(self))

def SETUP_FINALLY(self, inst):
self.block_stack.append(BlockStackEntry(inst.target))
self.block_stack.append(BlockStackEntry(0, inst.target))
Copy link
Collaborator

Choose a reason for hiding this comment

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

For all these functions that don't exist for 3.11, should we comment them the same way the ones only existing for python<=3.7 are?

@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
63a644a 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
fb38134 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Copy link
Member Author

@albanD I haven't had time to take a look at exception tables yet - I was planning on fixing it at the top of the PR stack.

@albanD
Copy link
Collaborator

albanD commented Feb 8, 2023

Delaying the exception table change sounds ok to me!

albanD
albanD approved these changes Feb 8, 2023
View reviewed changes
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Let's get this in!

@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
9bc7a81 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
47264cc 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
926166d 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
f90741c 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
41a3fc8 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
0a730c7 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
459f369 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
2d8be29 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
c0b53b0 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
a32f556 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
19e68a8 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@williamwen42
Update on "[dynamo 3.11] changes to with contexts"
2261640 
cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
@msaroufim msaroufim mentioned this pull request Mar 3, 2023
Closed
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Mar 5, 2023
@williamwen42 @cyyever
[dynamo 3.11] changes to with contexts (#94101)
5968c55 
Pull Request resolved: pytorch/pytorch#94101
Approved by: https://github.com/albanD, https://github.com/jansel
pruthvistony added a commit to ROCm/pytorch that referenced this pull request May 2, 2023
@pruthvistony
Revert "[dynamo 3.11] changes to with contexts (pytorch#94101)"
a4cadab 
This reverts commit 1123ab8.
@facebook-github-bot facebook-github-bot deleted the gh/williamwen42/35/head branch June 8, 2023 19:21
jhavukainen pushed a commit to kulinseth/pytorch that referenced this pull request Mar 15, 2024
@williamwen42 @jhavukainen
[dynamo 3.11] changes to with contexts (pytorch#94101)
85cdf47 
Pull Request resolved: pytorch#94101
Approved by: https://github.com/albanD, https://github.com/jansel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jansel jansel jansel approved these changes

@albanD albanD albanD approved these changes

@voznesenskym voznesenskym Awaiting requested review from voznesenskym

@mlazos mlazos Awaiting requested review from mlazos

@malfet malfet Awaiting requested review from malfet

@yanboliang yanboliang Awaiting requested review from yanboliang

Assignees
No one assigned
Labels
ciflow/inductor Merged module: dynamo release notes: dynamo
Projects
None yet
Milestone
Python 3.11
Development

Successfully merging this pull request may close these issues.
4 participants
@williamwen42 @albanD @jansel @pytorchmergebot