-
Notifications
You must be signed in to change notification settings - Fork 26.3k
[dynamo] fix keyerror in resume_execution, fix store attr #166924
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.
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
Conversation
Fixes #166033 Differential Revision: [D85198055](https://our.internmc.facebook.com/intern/diff/D85198055) Pull Request resolved: #166036 Approved by: https://github.com/Lucaskabela (cherry picked from commit ebb2b2e)
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/166924
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit abd9ef2 with merge base 4840a1a ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
66250f2 to
e328b4c
Compare
test/dynamo/test_repros.py
Outdated
| x = x + 8 | ||
| return x + 16 | ||
| x = x + 1 + 2 | ||
| torch._dynamo.step_unsupported() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @williamwen42 this doesn't exist on release/2.9... should we just remove this testcase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is coming from #162737 which we're not cherry picking? So we can probably just remove this test case.
e328b4c to
89b01de
Compare
| def find_orig_offset(cur_offset: int) -> int: | ||
| orig_offset = -1 | ||
|
|
||
| def find_orig_offset_transform( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @williamwen42 this code is also causing a breakage since (target,) is not able to be unpacked. Any thoughts on the right change to support this?
Fixes #166176 The error I attempted to fix in #162318 was still appearing internally. Surprised that this wasn't caught anywhere 😰 Pull Request resolved: #166040 Approved by: https://github.com/Lucaskabela ghstack dependencies: #166036 (cherry picked from commit 32fe4f6)
89b01de to
61e1943
Compare
atalman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Fixes #166176
See also stack on #166036
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames