Enabled add, sum and mul for bool tensor #21032

izdeby · 2019-05-28T18:48:59Z

Stack from ghstack:

Enable all and any for bool tensors #21033 Enable all and any for bool tensors
Enabled add, sum and mul for bool tensor #21032 Enabled add, sum, mul and neg for bool tensor
Enabled min, max, minall, maxall, cmin, cmax, cmaxValue, cminValue for bool tensors #21031 Enabled min, max, minall, maxall, cmin, cmax, cmaxValue, cminValue for bool tensors
Added possibility to index scalars by bool masks #21030 Added possibility to index scalars by bool masks

This PR is a part of a stack which will change result tensor type of comparison ops from uint8 to bool. As this change is rather big and a lot of prep work is needed, im breaking it into a stack.

Changes in this PR:

Enable add, sum, mul and neg for bool tensors.

Differential Revision: D15530496

colesbury

This needs a test for sum(). Where are the modifications for CUDA?

colesbury · 2019-05-28T19:09:42Z

aten/src/ATen/native/cpu/BinaryOpsKernel.cpp

 }

+template <typename T>
+T multiply(T a, T b) {


Try to avoid this. * should work on bool. You might need to add an overload to Vec256.

It would work without this but the build on CircleCi is failing due to an error about bad usage of "*" with two bools.

Then suppress the warning. I'm not a fan of unnecessarily verbose code driven by the compiler's preferences.

@cpuhrsch, made a good argument against adding an overload to Vec256.
Also, i dont feel comfortable with fully blocking warning for cases like this. If there was a way to block it for this method, i would be fine but as far as i can tell - we block gcc warnings for the whole project and i dont think its a good way to go.
Please, let me know if you have strong preferences here

#pragma GCC diagnostic ignored "-Wint-in-bool-context"

We should stick to the same operators because it keeps the code simple and reduces the chances of incorrect behavior. For example, this PR previously had an incorrect definition of - because it didn't use operator- -- it was the opposite of how - operates on a bool in both C/C++ and Python.

colesbury · 2019-05-28T19:14:25Z

aten/src/ATen/native/cpu/UnaryOpsKernel.cpp


 static void neg_kernel(TensorIterator& iter) {
-  AT_DISPATCH_ALL_TYPES(iter.dtype(), "neg_cpu", [&]() {
+  if (iter.dtype() == ScalarType::Bool) {


This doesn't look right. We shouldn't implement unary - on bool. We should implement bitwise not (unary ~) instead.

It's also worth noting that this is the opposite of C++ behavior. For example:

bool x = ... bool y = -x;

x and y have the same value (because 0=>0=>false and 1=>-1=>true)

removing this change for now. will implement this as a separate PR

colesbury · 2019-05-28T19:14:55Z

aten/src/THC/THCNumerics.cuh

-  static inline __host__ __device__ bool ge(uint8_t a, uint8_t b) { return a >= b; }
-  static inline __host__ __device__ bool eq(uint8_t a, uint8_t b) { return a == b; }
-  static inline __host__ __device__ bool ne(uint8_t a, uint8_t b) { return a != b; }
+  static inline __host__ __device__ bool min() { return at::numeric_limits<bool>::lowest(); }


Where is this used? We shouldn't be using THCNumerics in these functions any more and should try to avoid adding anything to it.

Please see the build error for a previous PR in the stack. I moved numerics changes to the previous PR.

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

ezyang · 2019-05-29T11:25:21Z

Don't forget to fix lint

VitalyFedyunin · 2019-05-29T15:18:17Z

aten/src/ATen/cpu/vec256/vec256_base.h

+template <> Vec256<bool> inline operator*(const Vec256<bool> &a, const Vec256<bool> &b) {
+  Vec256<bool> c = Vec256<bool>();
+  for (int i = 0; i != Vec256<bool>::size(); i++) {
+    c[i] = a[i] && b[i];


Not sure if it is going to work in your case, but please consider checking https://scc.ustc.edu.cn/zlsc/sugon/intel/compiler_c/main_cls/index.htm#intref_cls/common/intref_avx_fmadd_pd.htm

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

izdeby · 2019-05-29T22:15:33Z

@pytorchbot retest this please

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

izdeby · 2019-05-30T01:13:05Z

@pytorchbot retest this please

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Summary: Pull Request resolved: pytorch/pytorch#21032 ghimport-source-id: 6ab21752b4af451e8b10a0e02cd5d726aa7472f0 Differential Revision: D15530496 Pulled By: izdeby fbshipit-source-id: f4f83aa80eafbb4f307aadc1a13d8cdcf3055c24

facebook-github-bot · 2019-05-31T01:22:23Z

@izdeby merged this pull request in 9a22cb9.

gchanan · 2019-06-05T21:19:47Z

this added support for subtraction with bools (by accident?) but it's not tested anywhere.

Enabled add, sum, mul and neg for bool tensor

48f451b

pytorchbot added module: cpu CPU specific problem (e.g., perf, algorithm) module: cuda Related to torch.cuda, and CUDA support in general module: operators labels May 28, 2019

This was referenced May 28, 2019

Added possibility to index scalars by bool masks #21030

Closed

Enabled min, max, minall, maxall, cmin, cmax, cmaxValue, cminValue for bool tensors #21031

Closed

Enable all and any for bool tensors #21033

Closed

izdeby requested review from VitalyFedyunin, colesbury, ezyang and li-roy and removed request for colesbury May 28, 2019 19:08

colesbury reviewed May 28, 2019

View reviewed changes

Update on "Enabled add, sum, mul and neg for bool tensor"

cc093f5

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

izdeby changed the title ~~Enabled add, sum, mul and neg for bool tensor~~ [WIP] Enabled add, sum, mul and neg for bool tensor May 28, 2019

izdeby requested a review from colesbury May 28, 2019 21:39

izdeby added 2 commits May 28, 2019 15:38

Update on "[WIP] Enabled add, sum, mul and neg for bool tensor"

79e94c1

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "[WIP] Enabled add, sum, mul and neg for bool tensor"

f83d0b8

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

izdeby changed the title ~~[WIP] Enabled add, sum, mul and neg for bool tensor~~ [WIP] Enabled add, sum and mul for bool tensor May 28, 2019

izdeby changed the title ~~[WIP] Enabled add, sum and mul for bool tensor~~ Enabled add, sum and mul for bool tensor May 28, 2019

izdeby added 2 commits May 28, 2019 17:43

Update on "Enabled add, sum and mul for bool tensor"

662b987

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "Enabled add, sum and mul for bool tensor"

7f35234

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

ezyang removed their request for review May 29, 2019 11:25

VitalyFedyunin reviewed May 29, 2019

View reviewed changes

Update on "Enabled add, sum and mul for bool tensor"

7bb942e

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

izdeby requested a review from ezyang May 29, 2019 15:56

izdeby added 2 commits May 29, 2019 10:26

Update on "Enabled add, sum and mul for bool tensor"

41e5d8f

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "Enabled add, sum and mul for bool tensor"

035a8e4

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

VitalyFedyunin approved these changes May 29, 2019

View reviewed changes

Update on "Enabled add, sum and mul for bool tensor"

b984cc3

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

izdeby changed the title ~~Enabled add, sum and mul for bool tensor~~ [WIP] Enabled add, sum and mul for bool tensor May 29, 2019

izdeby added 2 commits May 29, 2019 14:18

Update on "[WIP] Enabled add, sum and mul for bool tensor"

8d672da

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "[WIP] Enabled add, sum and mul for bool tensor"

f4f2fe6

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

This was referenced May 29, 2019

Updated scalar type to onnx mapping #21095

Closed

Minor preparational JIT changes #21096

Closed

Update on "[WIP] Enabled add, sum and mul for bool tensor"

b4cfd70

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

izdeby changed the title ~~[WIP] Enabled add, sum and mul for bool tensor~~ Enabled add, sum and mul for bool tensor May 30, 2019

Update on "Enabled add, sum and mul for bool tensor"

ea9fedc

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

izdeby mentioned this pull request May 30, 2019

[BC-BREAKING] Changed tensor comparison return type from uint8 to bool #21113

Closed

izdeby added 9 commits May 29, 2019 19:43

Update on "Enabled add, sum and mul for bool tensor"

754365d

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "Enabled add, sum and mul for bool tensor"

d78a51d

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "Enabled add, sum and mul for bool tensor"

0dffc48

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "Enabled add, sum and mul for bool tensor"

ac27c51

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "Enabled add, sum and mul for bool tensor"

98b33f6

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "Enabled add, sum and mul for bool tensor"

7014134

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "Enabled add, sum and mul for bool tensor"

b33e9c3

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "Enabled add, sum and mul for bool tensor"

c16d798

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

Update on "Enabled add, sum and mul for bool tensor"

c263020

Enabled add, sum, mul and neg for bool tensor gh-metadata: pytorch pytorch 21032 gh/izdeby/3/head

facebook-github-bot closed this in 9a22cb9 May 30, 2019

zou3519 deleted the gh/izdeby/3/head branch May 30, 2019 23:14

facebook-github-bot added the merged label May 31, 2019

mruberry added the Merged label Oct 28, 2020

Enabled add, sum and mul for bool tensor #21032

Enabled add, sum and mul for bool tensor #21032

Uh oh!

Conversation

izdeby commented May 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

colesbury left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezyang commented May 29, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

izdeby commented May 29, 2019

Uh oh!

izdeby commented May 30, 2019

Uh oh!

facebook-github-bot commented May 31, 2019

Uh oh!

gchanan commented Jun 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

izdeby commented May 28, 2019 •

edited

Loading