Improve schedule load, add slice_like #1299

kevinthesun · 2018-06-20T00:50:29Z

Enhance x86 conv2d schedule loading to allow more flexible schedule loading patten.
Add crop_like operator.

srkreddy1238 · 2018-06-20T03:53:12Z

"Croping only 2nd and 3rd dimensions" and "Always crop based on other tensor shape" look like a special case consideration to me.

I suggest below changes for crop/crop_like for consideration to keep it more generalized.

TVM/TOPI interface could be (Input Tensor, Array of Offsets, Array Of Lengths) to keep it more independent.
At NNVM/TOP you could add logic to derive Lengths and Offsets based on inputs and attributes.
Along with crop_like you could add crop to nnvm which doesn't do anything but call tvm/topi.

Please await for others opinion ..

yzhliu · 2018-06-20T06:46:09Z

good to me for # 1 - Enhance x86 conv2d schedule loading to allow more flexible schedule loading pattern.

tqchen · 2018-06-21T02:39:46Z

reviewers, please try to make inline comments, or approve the change https://docs.tvm.ai/contribute/code_review.html#approve-and-request-changes-explicitly

srkreddy1238 · 2018-06-21T04:17:16Z

nnvm/include/nnvm/top/tensor.h

@@ -259,6 +259,17 @@ struct ClipParam : public dmlc::Parameter<ClipParam> {
  }
 };

+struct CropLikeParam : public dmlc::Parameter<CropLikeParam> {
+  Tuple<int> offsets;
+  bool center_crop;


Handle the layout as well.

The reason I didn't add layout as parameter is this operator's behavior is unrelated to layout. It just crops on the second and third axis.

srkreddy1238 · 2018-06-21T04:18:53Z

nnvm/src/top/tensor/transform.cc

+
+NNVM_REGISTER_OP(crop_like)
+.describe(R"code(Crop the 2nd and 3rd dim of input data,
+with width and height of the second input symbol.


A new operator to call tvm/crop directly

Issue for a single crop operator in nnvm is that number of inputs can be either 1 or 2. It requires an extra parameter to hint whether it is a "crop_like" operator to set number of inputs. I can add another crop operator in nnvm which just takes 1 input tensor.

srkreddy1238 · 2018-06-21T04:19:30Z

topi/include/topi/transform.h

+* \param name The name of the operation.
+* \param tag The tag to mark the operation.
+*
+* \return Cropped tensor


TVM/TOPI interface could be (Input Tensor, Array of Offsets, Array Of Lengths) to keep it more independent.

Is this related to the doc string?

tqchen · 2018-06-22T20:52:22Z

@kevinthesun can you act or make updated rely on the review comments?

tqchen · 2018-06-22T23:21:47Z

I am less sure about introducing crop operator, as crop seems to be a special case of slice operator( #1183), if shape inference can be done on the fly during conversion, it is likely we won't need the crop operator at all.

@kevinthesun maybe you can remove the crop related stuffs first and merge the rest part it?

kevinthesun · 2018-06-23T01:04:58Z

Yes. Slice is more general. I'll remove crop from topi.

tqchen · 2018-06-26T03:36:25Z

@srkreddy1238 please review again on the updated changes

srkreddy1238 · 2018-06-26T05:39:02Z

nnvm/src/top/tensor/transform.cc

+  if (param.axis.ndim() == 0) {
+    for (size_t i = 0; i < src_shape.ndim(); ++i) {
+      if (i < target_shape.ndim()) {
+        end_idx[i] = target_shape[i];


Add a check to make sure the end_idx doesn't go beyond input length.

srkreddy1238 · 2018-06-26T05:39:27Z

nnvm/src/top/tensor/transform.cc

+      CHECK_LT(i, target_shape.ndim())
+        << "Axis " << i << " exceeds dimension "
+        << target_shape.ndim()<< " of target_shape.";
+      end_idx[i] = target_shape[i];


Same check as above here as well.

srkreddy1238 · 2018-06-26T05:40:20Z

nnvm/src/top/tensor/transform.cc

+.describe(R"code(Slice the first input respect to the second input.
+)code" NNVM_ADD_FILELINE)
+.add_argument("data", "Tensor", "Input data to be sliced.")
+.add_argument("shape_like", "Tensor", "Shape like tensor")


shape_like or slice_like ?

srkreddy1238 · 2018-06-26T05:42:03Z

nnvm/src/top/tensor/transform.cc

+    Array<Expr> src_shape = inputs[0]->shape;
+    Array<Expr> target_shape = inputs[1]->shape;
+    Array<Expr> begin_idx, end_idx, strides;
+    for (size_t i = 0; i < src_shape.size(); ++i) {


Merge these two loops.

srkreddy1238 · 2018-06-26T05:42:38Z

nnvm/src/top/tensor/transform.cc

+        if (axis < 0) {
+          axis = static_cast<int>(src_shape.size()) + axis;
+        }
+        end_idx.Set(static_cast<size_t>(axis), target_shape[axis]);


Same check as above here as well.

srkreddy1238 · 2018-06-26T05:42:42Z

nnvm/src/top/tensor/transform.cc

+    if (param.axis.ndim() == 0) {
+      for (size_t i = 0; i < src_shape.size(); ++i) {
+        if (i < target_shape.size()) {
+          end_idx.Set(i, target_shape[i]);


Same check as above here as well.

srkreddy1238 · 2018-06-26T05:43:27Z

nnvm/src/top/tensor/transform.cc

+      topi::strided_slice(inputs[0], begin_idx, end_idx, strides)
+    };
+})
+.set_attr<FListInputNames>("FListInputNames", [](const NodeAttrs& attrs) {


shape_like or slice_like ?

srkreddy1238 · 2018-06-26T05:44:49Z

nnvm/tests/python/compiler/test_top_level4.py

+    slice_idx = []
+    for b, e in zip(begin_idx, end_idx):
+        slice_idx.append(slice(b, e))
+    np_result = np_data[slice_idx]


Make this numpy slicing as a function.

kevinthesun · 2018-06-27T00:24:45Z

@srkreddy1238 comment addressed.

srkreddy1238

@kevinthesun great thanks.

np_slice_like could be a sub function inside verify_slice_like as no other calls it.

Good to go now.

Wang and others added 4 commits June 19, 2018 17:03

Improve schedule loading

d9eefc6

Small fix

97dd2e4

FIx warning

c952850

Fix lint

10186cb

kevinthesun force-pushed the ImproveScheduleLoad branch from e1b6e36 to 10186cb Compare June 20, 2018 00:58

Fix from_mxnet

a11b2f7

kevinthesun force-pushed the ImproveScheduleLoad branch from f50c66c to a11b2f7 Compare June 20, 2018 01:05

srkreddy1238 requested changes Jun 21, 2018

View reviewed changes

tqchen added status: review in progress status: need update need update based on feedbacks labels Jun 21, 2018

Wang added 3 commits June 24, 2018 20:07

Change crop_like to slice_like

502ca4e

Fix warning

3695d38

Fix lint

94cfa24

tqchen changed the title ~~Improve schedule load~~ Improve schedule load, add slice_like Jun 26, 2018

srkreddy1238 requested changes Jun 26, 2018

View reviewed changes

Fix

bbb2fff

srkreddy1238 approved these changes Jun 27, 2018

View reviewed changes

tqchen added status: accepted and removed status: need update need update based on feedbacks status: review in progress labels Jun 27, 2018

tqchen merged commit c72cd71 into apache:master Jun 27, 2018

tqchen pushed a commit to tqchen/tvm that referenced this pull request Jul 6, 2018

Improve schedule load, add slice_like (apache#1299)

35b9eb1

mnuyens pushed a commit to mnuyens/tvm that referenced this pull request Jul 10, 2018

Improve schedule load, add slice_like (apache#1299)

71f8861

Laurawly mentioned this pull request Jul 12, 2018

[TOPI] Conv2d Schedule for Intel HD Graphics Target fails and produces wrong output #1420

Closed

kevinthesun deleted the ImproveScheduleLoad branch July 18, 2018 22:29

sergei-mironov pushed a commit to sergei-mironov/tvm that referenced this pull request Aug 8, 2018

Improve schedule load, add slice_like (apache#1299)

fe7fb79

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve schedule load, add slice_like #1299

Improve schedule load, add slice_like #1299

kevinthesun commented Jun 20, 2018

srkreddy1238 commented Jun 20, 2018

yzhliu commented Jun 20, 2018

tqchen commented Jun 21, 2018

srkreddy1238 Jun 21, 2018

kevinthesun Jun 22, 2018

srkreddy1238 Jun 21, 2018

kevinthesun Jun 22, 2018 •

edited

Loading

srkreddy1238 Jun 21, 2018

kevinthesun Jun 22, 2018

tqchen commented Jun 22, 2018

tqchen commented Jun 22, 2018 •

edited

Loading

kevinthesun commented Jun 23, 2018 •

edited

Loading

tqchen commented Jun 26, 2018

srkreddy1238 Jun 26, 2018

srkreddy1238 Jun 26, 2018

srkreddy1238 Jun 26, 2018

srkreddy1238 Jun 26, 2018

srkreddy1238 Jun 26, 2018

srkreddy1238 Jun 26, 2018

srkreddy1238 Jun 26, 2018

srkreddy1238 Jun 26, 2018

kevinthesun commented Jun 27, 2018

srkreddy1238 left a comment

Improve schedule load, add slice_like #1299

Improve schedule load, add slice_like #1299

Conversation

kevinthesun commented Jun 20, 2018

srkreddy1238 commented Jun 20, 2018

yzhliu commented Jun 20, 2018

tqchen commented Jun 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinthesun Jun 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tqchen commented Jun 22, 2018

tqchen commented Jun 22, 2018 • edited Loading

kevinthesun commented Jun 23, 2018 • edited Loading

tqchen commented Jun 26, 2018

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

kevinthesun commented Jun 27, 2018

srkreddy1238 left a comment

Choose a reason for hiding this comment

kevinthesun Jun 22, 2018 •

edited

Loading

tqchen commented Jun 22, 2018 •

edited

Loading

kevinthesun commented Jun 23, 2018 •

edited

Loading