8000 [Dynamo] Replace `unimplemented` with `unimplemented_v2` in `torch/_dynamo/variables/iter.py` by shink · Pull Request #151789 · pytorch/pytorch · GitHub
[go: up one dir, main page]

Skip to content

[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

Closed
wants to merge 6 commits into from

Conversation

shink
Copy link
Contributor
@shink shink commented Apr 21, 2025

Part of #147913

Replace unimplemented withunimplemented_v2 in torch/_dynamo/variables/iter.py

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

Copy link
pytorch-bot bot commented Apr 21, 2025

🔗 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 Failures

As of commit 4498c97 with merge base a0d440a (image):
💚 Looks good so far! There are no failures yet. 💚

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

@shink shink changed the title [Dynamo] Replace unimplemented with unimplemented_v2 in torch/_dynamo/variables/base.py [Dynamo] Replace unimplemented with unimplemented_v2 in torch/_dynamo/variables/iter.py Apr 21, 2025
@shink
Copy link
Contributor Author
shink commented Apr 21, 2025

@pytorchbot label "topic: not user facing"

@shink
Copy link
Contributor Author
shink commented Apr 21, 2025

@williamwen42 Could you please help review this? Thanks!

@shink
Copy link
Contributor Author
shink commented Apr 27, 2025

@pytorchbot rebase -b main

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/main. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased fix/dynamo/v2/iter onto refs/remotes/origin/main, please pull locally before adding more changes (for example, via git checkout fix/dynamo/v2/iter && git pull --rebase)

8000 @pytorchmergebot pytorchmergebot force-pushed the fix/dynamo/v2/iter branch from 93d1578 to af1c2eb Compare April 27, 2025 01:38
@shink
Copy link
Contributor Author
shink commented Apr 27, 2025

@williamwen42 Could you please take a look? Thanks for your time!

Comment on lines 107 to 111
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],
Copy link
Member

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.

Copy link
Contributor Author

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

unimplemented_v2(
gb_type="Unsupported `func` in itertools.accumulate",
context=f"call_function {self} {args} {kwargs}",
explanation="itertools.accumulate can only accept "
Copy link
Member

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).

Copy link
Contributor Author

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

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],
Copy link
Member

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

Copy link
Contributor Author

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

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))}",
Copy link
Member

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.)

Copy link
Contributor Author

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

@soulitzer soulitzer added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Apr 28, 2025
@williamwen42
Copy link
Member

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 28, 2025
@pytorchmergebot
Copy link
Collaborator

Merge started

Your 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

Advanced Debugging
Check the merge workflow status
here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged module: dynamo open source topic: not user facing topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0