-
Notifications
You must be signed in to change notification settings - Fork 24.2k
[Dynamo] Replace unimplemented
with unimplemented_v2
in torch/_dynamo/variables/iter.py
#151789
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
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/151789
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 4498c97 with merge base a0d440a ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
unimplemented
with unimplemented_v2
in torch/_dynamo/variables/base.py
unimplemented
with unimplemented_v2
in torch/_dynamo/variables/iter.py
@pytorchbot label "topic: not user facing" |
@williamwen42 Could you please help review this? Thanks! |
@pytorchbot rebase -b main |
@pytorchbot started a rebase job onto refs/remotes/origin/main. Check the current status here |
Successfully rebased |
93d1578
to
af1c2eb
Compare
@williamwen42 Could you please take a look? Thanks for your time! |
torch/_dynamo/variables/iter.py
Outdated
gb_type="Unsupported arguments for itertools.accumulate", | ||
context=f"call_function {self} {args} {kwargs}", | ||
explanation="Dynamo does not know how to trace " | ||
f"itertools.accumulate with args: {args}", | ||
hints=[*graph_break_hints.USER_ERROR], |
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.
We should mention that we expect args[0] to be a list-like object and there should be 1 or 2 positional args.
I don't think we should necessarily (only) tag this as user error since it may be the case that we don't have unpack_var_sequence
defined for args[0] even though the input object is list-like.
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.
Thanks for your review. Have updated in this commit: 32f625c
torch/_dynamo/variables/iter.py
Outdated
unimplemented_v2( | ||
gb_type="Unsupported `func` in itertools.accumulate", | ||
context=f"call_function {self} {args} {kwargs}", | ||
explanation="itertools.accumulate can only accept " |
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.
I think we would only reach here if the number of positional args is not 1 or 2. We should change this explanation (or better: refactor this check to actually check the right thing).
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.
Updated in this commit: 412e1e1
torch/_dynamo/variables/iter.py
Outdated
context=f"call_function {self} {args} {kwargs}", | ||
explanation="Dynamo does not know how to trace " | ||
f"itertools.groupby with args: {args}", | ||
hints=[*graph_break_hints.USER_ERROR], |
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.
Similar comment as in itertools.accumulate
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.
Updated in this commit: 71d6c49
torch/_dynamo/variables/iter.py
Outdated
gb_type="Unsupported key type for itertools.groupby", | ||
context=f"call_function {self} {args} {kwargs}", | ||
explanation="Dynamo does not know how to trace " | ||
f"itertools.groupby with key type: {str(type(key))}", |
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.
Mention that we only support grouping keys that are constants (int, float, str, etc.)
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.
Updated in this commit: 8269a85
@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 |
Part of #147913
Replace
unimplemented
withunimplemented_v2
intorch/_dynamo/variables/iter.py
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames