-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Restructure comparison ops so as to better support XLA dispatch #29591
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
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 had two inline questions but overall looks good to me! :D
Per @ailzhang's suggestion, this PR changes the implementation of binary comparison and logical ops to those of unary ops in UnaryOps.cpp. The reason is that the call should eventually go through at::op_out (e.g., at::logical_xor_out). The check for Boolean output tensor is also removed, because: - This check should only apply to _out functions but not on other variants. However, other variants must go through the _out variant eventually. - It does not have a clear motivation and seems unnecessary.
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.
@ailzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
All tests passed, landing. |
Summary: Per ailzhang's suggestion in pytorch/pytorch#28162 (comment), this PR changes the implementation of binary comparison and logical ops to those of unary ops in UnaryOps.cpp. The reason is that the call should eventually go through at::op_out (e.g., at::logical_xor_out). The check for Boolean output tensor is also removed, because: - This check should only apply to _out functions but not on other variants. However, other variants must go through the _out variant eventually. - It does not have a clear motivation and seems unnecessary. Pull Request resolved: pytorch/pytorch#29591 Differential Revision: D18460113 Pulled By: ailzhang fbshipit-source-id: 58d501e59335186b3b8cc7d80ee9eed74efeeac8
Emancipated by pytorch/pytorch#29591
|
Thanks! |
Emancipated by pytorch/pytorch#29591
Per @ailzhang's suggestion in #28162 (comment), this PR changes the implementation of binary comparison and logical ops
to those of unary ops in UnaryOps.cpp. The reason is that the call should eventually go through
at::op_out (e.g., at::logical_xor_out).
The check for Boolean output tensor is also removed, because:
must go through the _out variant eventually.