8000 [Set] Handle exception in ConstantVariable operation by guilhermeleobas · Pull Request #152987 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

[Set] Handle exception in ConstantVariable operation #152987

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
Closed
wants to merge 17 commits into from
Closed

[Set] Handle exception in ConstantVariable operation #152987

wants to merge 17 commits into from

Conversation

guilhermeleobas
Copy link
Collaborator
@guilhermeleobas guilhermeleobas commented May 6, 2025 •
edited
Loading

Stack from ghstack (oldest at bottom):

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

@guilhermeleobas
Update
bfd38b5 
[ghstack-poisoned]
@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented May 6, 2025 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

As of commit 1e1de9e with merge base 064f4c1 (image):
💚 Looks good so far! There are no failures yet. 💚

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

This was referenced May 6, 2025
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Draft
@guilhermeleobas
Update
9c6e1f8 
[ghstack-poisoned]
@guilhermeleobas guilhermeleobas marked this pull request as ready for review May 7, 2025 14:12
guilhermeleobas
guilhermeleobas commented May 7, 2025
View reviewed changes
torch/_dynamo/variables/builtin.py
explanation="Encountered exception when attempting to constant fold.",
hints=[*graph_break_hints.DYNAMO_BUG],
from_exc=exc,
raise_observed_exception(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@williamwen42, could you take a look at this change? Do you see any issues with replacing the unimplemented(...) call with raising an exception in Dynamo? If the exception isn't caught, it will propagate to CPython.

This change is needed to get some of the CPython set tests to pass.

@guilhermeleobas
Update
86ec969 
[ghstack-poisoned]
@guilhermeleobas
Update
f0d51cd 
[ghstack-poisoned]
@guilhermeleobas guilhermeleobas mentioned this pull request May 7, 2025
Closed
williamwen42
williamwen42 reviewed May 7, 2025
View reviewed changes
Copy link
Member
@williamwen42 williamwen42 left a comment

Choose a reason for hiding this comment

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

Looks generally fine, but slightly worried if there's a case where Dynamo raises an exception but eager code does not (can't think of one off the top of my head).

Do you have sample code that can be added to the unittests that demonstrates this fix?

@guilhermeleobas
Update
d86ff3a 
[ghstack-poisoned]
@guilhermeleobas
Update
e1e50f5 
[ghstack-poisoned]
@guilhermeleobas
Update
1b1902d 
[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Starting merge as part of PR stack under #152990

@guilhermeleobas
Update
143d9ee 
[ghstack-poisoned]
@guilhermeleobas
Update
0467a97 
[ghstack-poisoned]
@guilhermeleobas guilhermeleobas mentioned this pull request May 14, 2025
Draft
@guilhermeleobas
Update
af77358 
[ghstack-poisoned]
@guilhermeleobas
Update
a8f9da3 
[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Starting merge as part of PR stack under #152990

1 similar comment
@pytorchmergebot
Copy link
Collaborator

Starting merge as part of PR stack under #152990

@guilhermeleobas
Update
3becbce 
[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Starting merge as part of PR stack under #152908

@guilhermeleobas
Update
1e1de9e 
[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Starting merge as part of PR stack under #152990

pytorchmergebot pushed a commit that referenced this pull request May 16, 2025
@guilhermeleobas @pytorchmergebot
[Set] Raise TypeError if argument is unhashable (#152988)
481c345 
Pull Request resolved: #152988
Approved by: https://github.com/anijain2305
ghstack dependencies: #150792, #152987
pytorchmergebot pushed a commit that referenced this pull request May 16, 2025
@guilhermeleobas @pytorchmergebot
[Set] Raise TypeError if number of arguments mismatch (#152904)
fe51ce6 
Pull Request resolved: #152904
Approved by: https://github.com/anijain2305
ghstack dependencies: #150792, #152987, #152988
pytorchmergebot pushed a commit that referenced this pull request May 16, 2025
@guilhermeleobas @pytorchmergebot
[Set] Add set.symmetric_difference(_update) (#152901)
5926b7a 
Pull Request resolved: #152901
Approved by: https://github.com/anijain2305
ghstack dependencies: #150792, #152987, #152988, #152904
pytorchmergebot pushed a commit that referenced this pull request May 16, 2025
@guilhermeleobas @pytorchmergebot
[Set] Add set.issubset and set.issuperset (#152902)
574f4c5 
Pull Request resolved: #152902
Approved by: https://github.com/anijain2305
ghstack dependencies: #150792, #152987, #152988, #152904, #152901
pytorchmergebot pushed a commit that referenced this pull request May 16, 2025
@guilhermeleobas @pytorchmergebot
[Set] Raise KeyError if elem not contained in the set (#152903)
5c6830c 
Pull Request resolved: #152903
Approved by: https://github.com/anijain2305
ghstack dependencies: #150792, #152987, #152988, #152904, #152901, #152902
pytorchmergebot pushed a commit that referenced this pull request May 16, 2025
@guilhermeleobas @pytorchmergebot
[Set] Add set.difference(_update) (#152905)
ca96d55 
Pull Request resolved: #152905
Approved by: https://github.com/anijain2305
ghstack dependencies: #150792, #152987, #152988, #152904, #152901, #152902, #152903
pytorchmergebot pushed a commit that referenced this pull request May 16, 2025
@guilhermeleobas @pytorchmergebot
[Set] Add set.intersection(_update) (#152906)
4759922 
Pull Request resolved: #152906
Approved by: https://github.com/anijain2305
ghstack dependencies: #150792, #152987, #152988, #152904, #152901, #152902, #152903, #152905
pytorchmergebot pushed a commit that referenced this pull request May 16, 2025
@guilhermeleobas @pytorchmergebot
[Set] Update set.union and set.update to support *args (#152989)
5964cb5 
Pull Request resolved: #152989
Approved by: https://github.com/anijain2305
ghstack dependencies: #150792, #152987, #152988, #152904, #152901, #152902, #152903, #152905, #152906
pytorchmergebot pushed a commit that referenced this pull request May 16, 2025
@guilhermeleobas @pytorchmergebot
[Set] Raise KeyError on empty set.pop() (#152907)
0530254 
Pull Request resolved: #152907
Approved by: https://github.com/anijain2305
ghstack dependencies: #150792, #152987, #152988, #152904, #152901, #152902, #152903, #152905, #152906, #152989
pytorchmergebot pushed a commit that referenced this pull request May 16, 2025
@guilhermeleobas @pytorchmergebot
[Set] Add correct set/frozenset __init__ behavior (#152908)
5a0ca65 
Pull Request resolved: #152908
Approved by: https://github.com/anijain2305
ghstack dependencies: #150792, #152987, #152988, #152904, #152901, #152902, #152903, #152905, #152906, #152989, #152907
pytorchmergebot pushed a commit that referenced this pull request May 16, 2025
@guilhermeleobas @pytorchmergebot
[Set] Raise TypeError if set is called with the wrong number of argum…
f66a159 
…ents (#152990)

Pull Request resolved: #152990
Approved by: https://github.com/anijain2305
ghstack dependencies: #150792, #152987, #152988, #152904, #152901, #152902, #152903, #152905, #152906, #152989, #152907, #152908
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anijain2305 anijain2305 anijain2305 approved these changes

@williamwen42 williamwen42 williamwen42 approved these changes

@zou3519 zou3519 Awaiting requested review from zou3519 zou3519 is a code owner

@StrongerXi StrongerXi Awaiting requested review from StrongerXi

Assignees
No one assigned
Labels
ciflow/inductor Merged module: dynamo open source topic: not user facing topic category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@guilhermeleobas @pytorchmergebot @anijain2305 @williamwen42 @pytorchbot
0