Tensor roll op implementation #14953

kobejean · 2017-11-28T19:57:27Z

Closes #10761
Added a tf.manip.roll op that works similarly to numpy's np.roll. This was a feature requested in #10761 and was marked as contributions welcome.

Usage:

Rolls the elements of a tensor by the offsets of shift along the dimensions
of axis. Elements that roll passed the last position will wrap around
to the first.
For example:

# 't' is [0, 1, 2, 3, 4]
roll(t, shift=2, axis=0) ==> [3, 4, 0, 1, 2]
# shifting along multiple dimensions
# 't' is [[0, 1, 2, 3, 4], [5, 6, 7, 8, 9]]
roll(t, shift=[1, -2], axis=[0, 1]) ==> [[7, 8, 9, 5, 6], [2, 3, 4, 0, 1]]
# shifting along the same axis multiple times
# 't' is [[0, 1, 2, 3, 4], [5, 6, 7, 8, 9]]
roll(t, shift=[2, -3], axis=[1, 1]) ==> [[1, 2, 3, 4, 0], [6, 7, 8, 9, 5]]

shift: shift[i] specifies the number of places by which elements are shifted
along the dimension specified by axis[i]. Negative shifts will roll the
elements in the opposite direction.
axis: axis[i] specifies the dimension that the shift shift[i] should occur.
if the same axis is referenced more than once, the total shift for that axis
will be the sum of all the shifts that belong to that axis.
output: Has the same shape and size as the input. The elements are shifted by
the offsets of shift along the dimensions of axis.

Unit tests:

[==========] Running 24 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 24 tests from RollOpTest
[ RUN      ] RollOpTest.ScalarIndices
[       OK ] RollOpTest.ScalarIndices (5 ms)
[ RUN      ] RollOpTest.ScalarIndices_NoMemcpy
[       OK ] RollOpTest.ScalarIndices_NoMemcpy (0 ms)
[ RUN      ] RollOpTest.ScalarIndices_Complex
[       OK ] RollOpTest.ScalarIndices_Complex (0 ms)
[ RUN      ] RollOpTest.Simple_TwoD32
[       OK ] RollOpTest.Simple_TwoD32 (0 ms)
[ RUN      ] RollOpTest.Simple_TwoD32_NoMemcpy
[       OK ] RollOpTest.Simple_TwoD32_NoMemcpy (0 ms)
[ RUN      ] RollOpTest.Simple_ThreeD32
[       OK ] RollOpTest.Simple_ThreeD32 (1 ms)
[ RUN      ] RollOpTest.Simple_ThreeD32_NoMemcpy
[       OK ] RollOpTest.Simple_ThreeD32_NoMemcpy (0 ms)
[ RUN      ] RollOpTest.Simple_TwoD64
[       OK ] RollOpTest.Simple_TwoD64 (0 ms)
[ RUN      ] RollOpTest.Simple_TwoD64_NoMemcpy
[       OK ] RollOpTest.Simple_TwoD64_NoMemcpy (0 ms)
[ RUN      ] RollOpTest.Simple_ThreeD64
[       OK ] RollOpTest.Simple_ThreeD64 (0 ms)
[ RUN      ] RollOpTest.Simple_ThreeD64_NoMemcpy
[       OK ] RollOpTest.Simple_ThreeD64_NoMemcpy (0 ms)
[ RUN      ] RollOpTest.ZeroShift_ThreeD32
[       OK ] RollOpTest.ZeroShift_ThreeD32 (0 ms)
[ RUN      ] RollOpTest.ZeroShift_ThreeD32_NoMemcpy
[       OK ] RollOpTest.ZeroShift_ThreeD32_NoMemcpy (0 ms)
[ RUN      ] RollOpTest.ZeroSize_ThreeD32
[       OK ] RollOpTest.ZeroSize_ThreeD32 (0 ms)
[ RUN      ] RollOpTest.ZeroSize_ThreeD32_NoMemcpy
[       OK ] RollOpTest.ZeroSize_ThreeD32_NoMemcpy (0 ms)
[ RUN      ] RollOpTest.OneSize_ThreeD32
[       OK ] RollOpTest.OneSize_ThreeD32 (1 ms)
[ RUN      ] RollOpTest.OneSize_ThreeD32_NoMemcpy
[       OK ] RollOpTest.OneSize_ThreeD32_NoMemcpy (0 ms)
[ RUN      ] RollOpTest.DuplicateShifts_TwoD32
[       OK ] RollOpTest.DuplicateShifts_TwoD32 (0 ms)
[ RUN      ] RollOpTest.DuplicateShifts_TwoD32_NoMemcpy
[       OK ] RollOpTest.DuplicateShifts_TwoD32_NoMemcpy (0 ms)
[ RUN      ] RollOpTest.Error_InputMustBeVectorOrHigher
[       OK ] RollOpTest.Error_InputMustBeVectorOrHigher (0 ms)
[ RUN      ] RollOpTest.Error_AxisMustBeScalarOrVector
[       OK ] RollOpTest.Error_AxisMustBeScalarOrVector (0 ms)
[ RUN      ] RollOpTest.Error_ShiftMustBeScalarOrVector
[       OK ] RollOpTest.Error_ShiftMustBeScalarOrVector (0 ms)
[ RUN      ] RollOpTest.Error_ShiftAndAxisMustBeSameSize
[       OK ] RollOpTest.Error_ShiftAndAxisMustBeSameSize (0 ms)
[ RUN      ] RollOpTest.Error_AxisOutOfRange
[       OK ] RollOpTest.Error_AxisOutOfRange (0 ms)
[----------] 24 tests from RollOpTest (7 ms total)

[----------] Global test environment tear-down
[==========] 24 tests from 1 test case ran. (8 ms total)
[  PASSED  ] 24 tests.

//tensorflow/python/kernel_tests:manip_ops_test                          PASSED in 0.9s

Benchmarks:

Running main() from test_main.cc
Benchmark                     Time(ns) Iterations
-------------------------------------------------
BM_cpu_roll_outer/256/256        46739      16065	 5608.7MB/s 1402.2M items/s
BM_cpu_roll_outer/512/512       124247       5135	 8439.5MB/s 2109.9M items/s
BM_cpu_roll_outer/1024/1024     638245       1149	 6571.6MB/s 1642.9M items/s
BM_cpu_roll_outer/2048/2048    5261260        100	 3188.8MB/s 797.2M items/s
BM_cpu_roll_all/256/256         135631       5353	 1932.8MB/s 483.2M items/s
BM_cpu_roll_all/512/512         286927       2263	 3654.5MB/s 913.6M items/s
BM_cpu_roll_all/1024/1024       981417        727	 4273.7MB/s 1068.4M items/s
BM_cpu_roll_all/2048/2048      4549074        149	 3688.1MB/s 922.0M items/s

I had made a pull request for this before but accidentally closed it

Half migrated to tf.manip

Added compile script

fix namespace error

Fixed thread pooling issue that was due to a bad cost_per_unit parameter Also improved readability

DoRollV2 copies memory in groups instead of element by element. Not thoroughly tested yet. Polished DoRollV2 algorithm

Roll op GPU bug fix GPU bug fix Roll op GPU bug fix Roll op GPU fix Roll GPU test BUILD update

Fully remove roll op GPU code Remove compile_cpu.sh

… bug

Migrated to tf.manip Roll op registered Roll op uses InlinedVector Small improvements

tensorflow-jenkins · 2017-11-28T19:57:28Z

Can one of the admins verify this patch?

kobejean · 2017-12-23T03:33:22Z

@yzhwang Thanks for the review! Just added the changes!

rmlarsen · 2018-01-23T18:42:47Z

@yzhwang Could you please take another look?

yzhwang

Thanks for the change! Both code and tests LGTM, just had some minor nits, all should be easy fix. Also please resolve on conflict in the BUILD file before you merge in the code.

yzhwang · 2018-01-24T21:42:29Z

tensorflow/cc/ops/op_gen_overrides.pbtxt

@@ -0,0 +1,237 @@
+# array_ops


Note that this file has been deleted with this commit, please do a sync:
d6326d7#diff-c35db0b1af76a093b0c04498d159994a

yzhwang · 2018-01-24T21:46:33Z

tensorflow/core/api_def/base_api/api_def_Roll.pbtxt

+op {
+  graph_op_name: "Roll"
+  in_arg {
+    name: "shift"


Please specify that the number of dimensions for both shift and axis.

yzhwang · 2018-01-24T22:10:46Z

tensorflow/core/kernels/roll_op.cc

@@ -0,0 +1,334 @@
+/* Copyright 2015 The TensorFlow Authors. All Rights Reserved.


yzhwang · 2018-01-24T22:15:11Z

tensorflow/core/kernels/roll_op.cc

+            // offset = shift[j] + ... other offsets
+            // offset - dim_range[j] = -threshold[j] + ... other offsets
+            // thus we undo our previous offset as well as add a new offset of
+            // -threshold[j] in one opperation


opperation->operation

yzhwang · 2018-01-24T22:17:03Z

tensorflow/core/kernels/roll_op_test.cc

@@ -0,0 +1,481 @@
+/* Copyright 2015 The TensorFlow Authors. All Rights Reserved.


yzhwang · 2018-01-24T22:27:42Z

tensorflow/core/kernels/roll_op_test.cc

+  test::ExpectTensorEqual<string>(expected, *GetOutput(0));
+}
+
+TEST_F(RollOpTest, DuplicateShifts_TwoD32) {


Consider switching duplicate to multi since you are testing combining multiple shifts on one axis.

yzhwang · 2018-01-24T22:38:08Z

tensorflow/core/kernels/roll_op_test.cc

+  EXPECT_TRUE(StringPiece(s.ToString()).contains("is out of range")) << s;
+}
+
+static Graph* RollGraph(const TensorShape& shape, int isd) {


What is isd here?

I'll add some comments to clarify. This also stands for inner shift dimension. Basically I shift the isd and all outer dimensions. When I benchmark with a 2-D tensor if isd=0 then since dimension 1 is on the inside of dimension 0, dimension 1 will not be shifted and dimension 0 will. If isd=1 then both dimension 1 and 0 get shifted. This allows me to compare performance between just shifting the outer dimension and shifting all dimensions.

yzhwang · 2018-01-24T22:39:25Z

tensorflow/core/ops/manip_ops.cc

@@ -0,0 +1,67 @@
+/* Copyright 2016 The TensorFlow Authors. All Rights Reserved.


yzhwang · 2018-01-24T22:42:27Z

tensorflow/python/kernel_tests/manip_ops_test.py

@@ -0,0 +1,126 @@
+# Copyright 2015 The TensorFlow Authors. All Rights Reserved.


kobejean · 2018-01-25T04:17:21Z

Thanks for the review! Just added the last changes!

Roll op: fixed BUILD file Roll op: api docs update

- updates goldens and fixes api compatibility issue - fixes python op test issue for windows - fixes makefile issues

kobejean · 2018-01-25T21:33:41Z

@rmlarsen I think I fixed the failures. Can we run the checks again?

Windows CMake checks were failing because numpy was on an older version that did not support np.roll on multiple shifts that was use to check the correctness of tf.manip.roll. manip_ops_test.py now checks for numpy version 1.12.0 before testing multiple shifts, otherwise it'll just test single shift roll.

kobejean · 2018-01-25T23:13:34Z

@rmlarsen @yzhwang Ok this should do it. Sorry can we run the checks again?

rmlarsen · 2018-01-30T00:03:23Z

@asimshankar thanks. Sorry I missed that you had already done an API review on this!

kobejean added 23 commits October 14, 2017 16:27

Half migrated to manip

7bd8864

Half migrated to tf.manip

Roll op: polymorphism & GPU attempt

6abeb5d

Roll op: Added support for gradients

59584b8

Added compile script

Rebase for roll op code

159672c

Roll op: Migrated to manip namespace

8ff2efe

Roll op: Supports CPU thread pooling

f252352

fix namespace error

Remove roll from user_ops

ec19e40

Roll op: Optimization

fde12fb

Roll op: Pylint fix

937e029

Roll op: Updated documentation

e4a56ba

Roll op: Two versions for CPU thread pooling

8d5e237

Roll op: Huge CPU speed up

f5cd08d

Fixed thread pooling issue that was due to a bad cost_per_unit parameter Also improved readability

Roll op: Rough draft of DoRollV2

0f59531

DoRollV2 copies memory in groups instead of element by element. Not thoroughly tested yet. Polished DoRollV2 algorithm

Roll op: Restrict tensor size for GPU implementation

b43b1fc

Roll op: Fixed clang-format and missing include

83ff035

Roll op: Minor change

c796b1b

Roll op GPU bug fix

22cda8c

Roll op GPU bug fix GPU bug fix Roll op GPU bug fix Roll op GPU fix Roll GPU test BUILD update

Roll op: Remove GPU code

2de639e

Fully remove roll op GPU code Remove compile_cpu.sh

Roll op: Fixes problems with array_ops_test.py and a size 1 dimension…

888bc80

… bug

Roll op: Migrated to manip

7278d71

Migrated to tf.manip Roll op registered Roll op uses InlinedVector Small improvements

Roll op: Revert array op changes

bbfd685

Roll op: Api def fix

3e58ed4

Merge branch 'roll'

a72802d

googlebot added the cla: yes label Nov 28, 2017

sb2nov requested a review from jhseu November 29, 2017 15:59

sb2nov added awaiting review Pull request awaiting review kokoro:run labels Nov 29, 2017

kokoro-team removed the kokoro:run label Nov 29, 2017

Merge branch 'master' into master

d432df4

kobejean added 4 commits January 15, 2018 11:26

Roll op: resolve conflicts

5d6da1b

Roll op: Resolve conflicts

754ddaf

Merge remote-tracking branch 'upstream/master' into resolve-m

0fbcce2

Roll op: clang-tidy

b27dd61

rmlarsen self-assigned this Jan 23, 2018

yzhwang approved these changes Jan 24, 2018

View reviewed changes

rmlarsen added API review API Review stat:awaiting response Status - Awaiting response from author and removed awaiting review Pull request awaiting review labels Jan 24, 2018

Merge remote-tracking branch 'upstream/master'

711c38d

Roll op: Review round 2 changes

b4f0232

Roll op: fixed BUILD file Roll op: api docs update

kobejean force-pushed the master branch from 785cd8f to b4f0232 Compare January 25, 2018 06:50

rmlarsen added kokoro:run and removed stat:awaiting response Status - Awaiting response from author labels Jan 25, 2018

kokoro-team removed the kokoro:run label Jan 25, 2018

Roll op: failure fixes 1

72ed7e2

- updates goldens and fixes api compatibility issue - fixes python op test issue for windows - fixes makefile issues

yzhwang added the kokoro:run label Jan 25, 2018

kokoro-team removed the kokoro:run label Jan 25, 2018

Roll op: pylint changes

70fc4c0

yzhwang added the kokoro:run label Jan 25, 2018

kokoro-team removed the kokoro:run label Jan 25, 2018

asimshankar removed the API review API Review label Jan 29, 2018

rmlarsen merged commit 4ab2d85 into tensorflow:master Jan 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tensor roll op implementation #14953

Tensor roll op implementation #14953

kobejean commented Nov 28, 2017 •

edited

tensorflow-jenkins commented Nov 28, 2017

kobejean commented Dec 23, 2017

rmlarsen commented Jan 23, 2018

yzhwang left a comment

yzhwang Jan 24, 2018

yzhwang Jan 24, 2018

yzhwang Jan 24, 2018

yzhwang Jan 24, 2018

yzhwang Jan 24, 2018

yzhwang Jan 24, 2018

yzhwang Jan 24, 2018

kobejean Jan 25, 2018

yzhwang Jan 24, 2018

yzhwang Jan 24, 2018

kobejean commented Jan 25, 2018

kobejean commented Jan 25, 2018

kobejean commented Jan 25, 2018

rmlarsen commented Jan 30, 2018

		@@ -0,0 +1,334 @@
		/* Copyright 2015 The TensorFlow Authors. All Rights Reserved.

		@@ -0,0 +1,481 @@
		/* Copyright 2015 The TensorFlow Authors. All Rights Reserved.

		@@ -0,0 +1,67 @@
		/* Copyright 2016 The TensorFlow Authors. All Rights Reserved.

		@@ -0,0 +1,126 @@
		# Copyright 2015 The TensorFlow Authors. All Rights Reserved.

Tensor roll op implementation #14953

Tensor roll op implementation #14953

Conversation

kobejean commented Nov 28, 2017 • edited

Usage:

Unit tests:

Benchmarks:

tensorflow-jenkins commented Nov 28, 2017

kobejean commented Dec 23, 2017

rmlarsen commented Jan 23, 2018

yzhwang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kobejean commented Jan 25, 2018

kobejean commented Jan 25, 2018

kobejean commented Jan 25, 2018

rmlarsen commented Jan 30, 2018

kobejean commented Nov 28, 2017 •

edited