Add TORCH_CHECK to disable sub for bool tensors #23335
Conversation
pytorchbot
added
module: cpu
gchanan
left a comment
gchanan
left a comment
There was a problem hiding this comment.
we should have a real error message that indicates we don't plan to support bool; as it is currently it can seem like a mistake.
xuhdev
force-pushed
the
op/sub-dispatch
branch
2 times, most recently
from
3162916 to
7daa610
Compare
izdeby
left a comment
izdeby
left a comment
There was a problem hiding this comment.
Can you, please, add tests?
colesbury
left a comment
colesbury
left a comment
There was a problem hiding this comment.
This seems unnecessarily complicated to me:
The check for subtraction on bool data dtypes should just be in BinaryOps.cpp (not in the kernels). A
TORCH_CHECK(iter.dtype() != kBool, "sub is not supported on bool tensors");
should suffice.
Please do not template these kernels on "bool add".
xuhdev
changed the title
xuhdev
force-pushed
the
op/sub-dispatch
branch
2 times, most recently
from
7013b6a to
32f9ae4
Compare
xuhdev
force-pushed
the
op/sub-dispatch
branch
4 times, most recently
from
d0570da to
6b5e9c4
Compare
|
@pytorchbot retest this please |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@izdeby has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
colesbury
added
the
merge-this-please
aten/src/ATen/native/BinaryOps.cpp
Outdated
There was a problem hiding this comment.
this doesn't look correct. ~ is not the correct suggestion for boolTensor - boolTensor (there are two boolean operands!), but it is temporarily for "1 - boolTensor", which is intended to invert the values.
There was a problem hiding this comment.
How about this:
if both operands are bool:
your error message, but with `^` instead as the suggestion.
if one operand is bool:
"Subtraction, the `-` operator, with a bool tensor is not supported. If you are trying to invert a mask, use the `~` or `bitwise_not()` operator instead."
|
@xuhdev are you going to adjust the error message for |
|
@colesbury I think I messed git on my side. Will reopen a PR. |
7 participants
This resolves two issues in one shot:
shows "add_cpu/add_cuda is not implemented for [type]". They should be
"sub_cpu/sub_cuda" instead.