-
Notifications
You must be signed in to change notification settings - Fork 26.3k
[dynamo] Remove extra if statement in builder _wrap #161215
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/161215
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 1 PendingAs of commit fad538a with merge base 7006fd0 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
StrongerXi
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.
Thanks!
torch/_dynamo/variables/builder.py
Outdated
| ): | ||
| # If it's either tensor or subclass with default torch_dispatch | ||
| # (they might override torch_function), we can always trace into them. | ||
| # For non-default torch_dispatch, we have more requirements. |
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.
While you are here, maybe remove this comment as well, it's confusing, it was meant to go before the is_traceable_wrapper_subclass(value) check, as that check encodes the "more requirements".
The comment above this basically elaborates on the type(value).__torch_dispatch__ is torch.Tensor.__torch_dispatch__ check.
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.
Sg, removed. Will merge after CI is green.
|
@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 |
Removes a redundant if statement. Does not impact logic so no test changes needed. Pull Request resolved: pytorch#161215 Approved by: https://github.com/StrongerXi
Removes a redundant if statement. Does not impact logic so no test changes needed.
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames @Lucaskabela @mlazos