Make any and all on ByteTensor behave like sum/prod. #4627

sighingnow · 2018-01-12T06:46:32Z

Make ByteTensor's methods any and all can accept dim and keepdim parameters, behave like sum/prod.

I add two macro TENSOR_IMPLEMENT_ACC and TENSOR_IMPLEMENT_ACCALL to make the code less duplicate.

This feature should fix issue #4313

pytorchbot · 2018-01-12T06:46:32Z

@sighingnow, thanks for your PR! We identified @zdevito to be a potential reviewer.

apaszke

That looks good, thanks for the PR. I only have a few minor comments

aten/src/TH/generic/THTensorMath.c

-    THTensor_(squeeze1d)(r_, r_, dimension);
-  }
-}
+#define TENSOR_IMPLEMENT_ACC(NAME, ACC_OP, INIT_VALUE)                                   \


aten/src/TH/generic/THTensorMath.c

-    TH_TENSOR_APPLY(real, tensor, sum = sum OP *tensor_data;); \
-    return sum; \
-  }
+TENSOR_IMPLEMENT_ACCALL(logicalallall, int, &&, 1)


test/test_torch.py

+    @unittest.skipIf(not TEST_NUMPY, "Numpy not found")
+    def test_all_any_with_dim(self):
+        def test(x):
+            r1 = np.all(x, axis=1, keepdims=False).astype('uint8')


torch/lib/THD/master_worker/common/Functions.hpp

  tensorLogspace,
  tensorRand,
  tensorRandn,
+  tensorLogicalallall,


torch/csrc/generic/methods/TensorCompare.cwrap

+        - bool keepdim
+    - cname: logicalany
+      return: argument 0
+      before_call: maybeThrowBackCompatKeepdimWarn("any");


sighingnow · 2018-01-12T12:47:30Z

Thanks for your review, @apaszke , new patch done.

gchanan

looks good, thanks! Some small suggestions below, and I'll fix the general ATen issue about bool returns.

aten/src/TH/generic/THTensorMath.c

+// Reduce all elements in the given tensor `t` with given `INIT_VALUE` and return
+// the result of type `RETTYPE`.
+// The accumulate operator `ACC_OP` must be commutative.
+#define TENSOR_IMPLEMENT_ACCALL(NAME, RETTYPE, ACC_OP, INIT_VALUE)        \


aten/src/TH/generic/THTensorMath.c


-TENSOR_IMPLEMENT_LOGICAL_SUM(logicalall, &&, 1)
-TENSOR_IMPLEMENT_LOGICAL_SUM(logicalany, ||, 0)
+TENSOR_IMPLEMENT_ACC(logicalAnd, &&, 1)


test/test_torch.py

+            self.assertEqual(r3.shape, r4.shape)
+            self.assertTrue((r3 == r4).all())
+
+        test(torch.rand((1, 2, 3, 4)).round().byte())


aten/src/ATen/Declarations.cwrap

-    - THTensor* self
+  options:
+    - cname: logicalAndAll
+      return: bool


gchanan · 2018-01-12T17:48:44Z

Ok, #4639 should fix the bool return issue. Let's merge that first and rebase this one so the return type is always consistent.

gchanan · 2018-01-12T19:26:04Z

#4639 is in.

Signed-off-by: HE, Tao <sighingnow@gmail.com>

sighingnow · 2018-01-15T02:44:51Z

Ping @gchanan

ezyang · 2018-03-23T21:42:39Z

This needs a rebase now; sorry about the delay :(

* upstream/master: (663 commits) Fix "command not found" error in perf test (pytorch#5982) add pip mkl-devel to the error message when mkl is found but mkl headers are not (pytorch#5984) Support batch LowerCholeskyTransform (pytorch#5980) Linearly interpolating upsampling fix (pytorch#5927) Store perf numbers in S3 (pytorch#5951) Modidy setup docs for Windows (pytorch#5981) Group Normalization (pytorch#5968) [distributions] Implement Power transform (pytorch#5976) Disable TestBottleneck test_cuda on Windows (pytorch#5977) Fix crash when cat-ing empty cuda tensors (pytorch#5971) Update no_unions flag for nanopb gen and update ONNX proto files (pytorch#5972) Expose gradients w.r.t. input & weight for conv1d, conv2d, conv3d in Python (pytorch#5408) Fixed non-determinate preprocessing on DataLoader (pytorch#4640) add AVX2 implementation for sigmoid function (pytorch#5010) Implement torch.util.bottleneck (pytorch#5216) Remove pragma once from cpp file (pytorch#5965) fix mvn docs (pytorch#5967) Fix incorrect rendering of Tensor.index_*_ doc examples. (pytorch#5969) Implement range for loop in script (pytorch#5827) Add windows doc (pytorch#5859) ... # Conflicts: # aten/src/TH/generic/THTensorMath.c # torch/_tensor_docs.py # torch/csrc/generic/methods/TensorCompare.cwrap

Signed-off-by: HE, Tao <sighingnow@gmail.com>

sighingnow · 2018-03-25T11:00:18Z

Thanks for picking up this. @ezyang I have rebased this PR to current master.

ezyang · 2018-03-26T15:48:42Z

@pytorchbot retest this please

sighingnow · 2018-03-27T02:39:53Z

Fix compile error in THD module.

ezyang · 2018-03-29T18:56:53Z

@pytorchbot retest this please

sighingnow · 2018-03-30T02:13:02Z

The failures wasn't caused by the code. Seems something bad happened to the CI server.

ezyang · 2018-03-30T19:38:40Z

@pytorchbot retest this please

sighingnow · 2018-04-25T07:17:49Z

Any further comments on this patch ? I think it's now ready to get merged!

/cc @ezyang

aten/src/TH/generic/THTensorMath.c

+  return (bool)sum;
+}
+
+void THTensor_(logicalAnd)(THTensor *r_, THTensor *t, int dimension, int keepdim)


soumith · 2018-04-25T17:09:49Z

15:08:33 ======================================================================
15:08:33 ERROR: test_all_any_with_dim (test_torch.TestTorch)
15:08:33 ----------------------------------------------------------------------
15:08:33 Traceback (most recent call last):
15:08:33   File "test_torch.py", line 794, in test_all_any_with_dim
15:08:33     [1, 1, 1]]))
15:08:33   File "test_torch.py", line 784, in test
15:08:33     self.assertTrue((r1 == r2).all())
15:08:33 RuntimeError: Expected object of type torch.LongTensor but found type torch.ByteTensor for argument #2 'other'
15:08:33 
15:08:33 ----------------------------------------------------------------------

This broken master

sighingnow · 2018-04-26T00:09:32Z

Sorry for the breakage. Thanks for fixing this @soumith .

apaszke reviewed Jan 12, 2018

View reviewed changes

gchanan reviewed Jan 12, 2018

View reviewed changes

sighingnow added 5 commits January 13, 2018 12:37

Make any and all on ByteTensor behave like sum/prod.

31bcb76

Signed-off-by: HE, Tao <sighingnow@gmail.com>

Rename logicalall to logicalAnd and improve the test case.

b0d8713

Signed-off-by: HE, Tao <sighingnow@gmail.com>

Fix typo in doc.

a5ce177

Signed-off-by: HE, Tao <sighingnow@gmail.com>

Fix a typo.

6286ce7

Signed-off-by: HE, Tao <sighingnow@gmail.com>

Update test case for any/all with hardcode input matrix.

560c524

Signed-off-by: HE, Tao <sighingnow@gmail.com>

sighingnow force-pushed the all-any branch from b3a6669 to 560c524 Compare January 13, 2018 06:19

Reanme ACC to REDUCE.

2d4051d

Signed-off-by: HE, Tao <sighingnow@gmail.com>

sighingnow force-pushed the all-any branch from 0f7ead7 to 557c55d Compare March 25, 2018 06:43

Fix return type.

64a3fbf

Signed-off-by: HE, Tao <sighingnow@gmail.com>

Fix compile error in THD module.

410f153

apaszke approved these changes Apr 25, 2018

View reviewed changes

apaszke merged commit 39d4814 into pytorch:master Apr 25, 2018

sighingnow deleted the all-any branch April 25, 2018 09:36

avmgithub mentioned this pull request Apr 25, 2018

[pytorch ppc64le] build break for ppc64le could be related to #4627 #6944

Closed

soumith mentioned this pull request Apr 25, 2018

sum/prod returns LongTensor, make test expect that #6957

Merged

Jorghi12 pushed a commit to wsttiger/pytorch that referenced this pull request May 10, 2018

Make any and all on ByteTensor behave like sum/prod. (pytorch#4627)

5459274

weiyangfb pushed a commit to weiyangfb/pytorch that referenced this pull request Jun 11, 2018

Make any and all on ByteTensor behave like sum/prod. (pytorch#4627)

0826202

ezyang added the open source label Jun 24, 2019

Make any and all on ByteTensor behave like sum/prod. #4627

Make any and all on ByteTensor behave like sum/prod. #4627

Uh oh!

Conversation

sighingnow commented Jan 12, 2018

Uh oh!

pytorchbot commented Jan 12, 2018

Uh oh!

apaszke left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

sighingnow commented Jan 12, 2018

Uh oh!

gchanan left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

gchanan commented Jan 12, 2018

Uh oh!

gchanan commented Jan 12, 2018

Uh oh!

sighingnow commented Jan 15, 2018

Uh oh!

ezyang commented Mar 23, 2018

Uh oh!

sighingnow commented Mar 25, 2018

Uh oh!

ezyang commented Mar 26, 2018

Uh oh!

sighingnow commented Mar 27, 2018

Uh oh!

ezyang commented Mar 29, 2018

Uh oh!

sighingnow commented Mar 30, 2018

Uh oh!

ezyang commented Mar 30, 2018

Uh oh!

sighingnow commented Apr 25, 2018

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

soumith commented Apr 25, 2018

Uh oh!

sighingnow commented Apr 26, 2018

Uh oh!