Skip to content
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

Either support torch.mean on BoolTensors or fix the error message #64897

Open
vadimkantorov opened this issue Sep 12, 2021 · 6 comments
Open
Labels
module: boolean tensor module: error checking Bugs related to incorrect/lacking error checking module: reductions triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@vadimkantorov
Copy link
Contributor

vadimkantorov commented Sep 12, 2021

(Pdb) torch.tensor(True).mean()
*** RuntimeError: Can only calculate the mean of floating types. Got Long instead.
(Pdb) torch.tensor([True, False]).mean()
*** RuntimeError: Can only calculate the mean of floating types. Got Long instead.

I guess this is because torch.mean first upcasts inputs to torch.int64 and then fails. But this is quite confusing + uses outdated dtype names (I propose error messages should use torch.int64 ).

For int64 mean is also not supported:

(Pdb) torch.tensor([1, 2]).mean()
*** RuntimeError: Can only calculate the mean of floating types. Got Long instead.

It would also make sense if torch.mean was supported by torch.int64 (hopefully without copy allocation of float cast, but maybe for now this is not possible) without an explicit cast

This is also quite wasteful to copy-allocate torch.bool to torch.int64 to perform these basic operations :( Related: #55366

cc @heitorschueroff @brianjo @mruberry

@jbschlosser jbschlosser added module: boolean tensor module: docs Related to our documentation, both in docs/ and docblocks triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module module: error checking Bugs related to incorrect/lacking error checking and removed module: docs Related to our documentation, both in docs/ and docblocks labels Sep 13, 2021
@mruberry
Copy link
Collaborator

We would accept a PR updating mean participate in integer->floating point type promotion and a more short-term fix for the error message.

Given PyTorch's current architecture taking the mean of a long tensor would probably require making a float copy because of build size concerns. On CUDA we might be able to fuse the copy into the kernel and not actually materialize the float copy, however.

@vadimkantorov
Copy link
Contributor Author

vadimkantorov commented Sep 16, 2021

As dicussion for more long-termed solution, I wonder if JIT could produce these inputXoutput dtype combinations code? (including CPU?)

@mruberry
Copy link
Collaborator

As dicussion for more long-termed solution, I wonder if JIT could produce these inputXoutput dtype combinations code? (including CPU?)

Sure; if you have a CPU jit than can fuse pointwise operations into the prologue of reductions, which is a pretty common fusion scenario.

@vadimkantorov
Copy link
Contributor Author

It may prevent a vectorized read though?

@mruberry
Copy link
Collaborator

It may prevent a vectorized read though?

It's hard to speculate on the details of hypothetical systems

@vadimkantorov
Copy link
Contributor Author

Similar error for uint8 dtype: *** RuntimeError: Can only calculate the mean of floating types. Got Long instead.. At the very least the error message is confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: boolean tensor module: error checking Bugs related to incorrect/lacking error checking module: reductions triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

No branches or pull requests

4 participants