You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/07/02 18:43:35 UTC
[GitHub] [incubator-tvm] mbrookhart opened a new pull request #5983: Dynamic Tile Op
mbrookhart opened a new pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983
@icemelon9 @kevinthesun
FYI - I'm working on gradients for these ops, need to refactor the autodiff testing a little bit to ignore these dynamic inputs, they shouldn't be differentiated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450371610
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -46,6 +47,14 @@ class DynamicToStaticMutator : public MixedModeMutator {
static const Op& reshape = Op::Get("reshape");
return Call(reshape, {call_node->args[0]}, Attrs(attrs), {});
}
+ } else if (call_node->op == dyn_tile_op_) {
Review comment:
yeah, thanks, I am oaky either way. Another question, what will happen if users forget to do this conversion? Will the program crash or proceed with the dynamic op?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450403162
##########
File path: python/tvm/relay/op/dyn/_transform.py
##########
@@ -81,3 +84,23 @@ def _reshape_shape_func_input_data(data, newshape, ndim):
@_reg.register_shape_func("dyn.reshape", True)
def dynamic_reshape_shape_func(attrs, inputs, out_ndims):
return [_reshape_shape_func_input_data(*inputs, out_ndims[0])]
+
+
+@script
+def _tile_shape_func(data, reps, ndim):
+ out = output_tensor((ndim,), "int64")
+
+ for i in const_range(ndim):
+ out[i] = data.shape[i] * int64(reps[i])
+ return out
+
+
+@_reg.register_shape_func("dyn.tile", True)
+def tile_shape_func(attrs, inputs, _):
+ """
+ Shape function for tile op.
+ """
+ ndim = len(inputs[0].shape)
+ rdim = inputs[1].shape[0].value
+ assert ndim == rdim, "tile data and reps ranks don't match"
Review comment:
I'm thinking about that more this morning, I'm not sure it's 100% required to prevent dynamic ranks, I'm wondering if I can expand it back to cover the other definition.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] yongwww commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
yongwww commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450505449
##########
File path: src/relay/op/dyn/tensor/transform.cc
##########
@@ -128,6 +130,71 @@ RELAY_REGISTER_OP("dyn.reshape")
.set_attr<FTVMCompute>("FTVMCompute", ReshapeCompute)
.set_attr<TOpPattern>("TOpPattern", kInjective);
+// tile operator
+// TVM_REGISTER_NODE_TYPE(TileAttrs);
Review comment:
```suggestion
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450402665
##########
File path: python/tvm/relay/op/dyn/_transform.py
##########
@@ -81,3 +84,23 @@ def _reshape_shape_func_input_data(data, newshape, ndim):
@_reg.register_shape_func("dyn.reshape", True)
def dynamic_reshape_shape_func(attrs, inputs, out_ndims):
return [_reshape_shape_func_input_data(*inputs, out_ndims[0])]
+
+
+@script
+def _tile_shape_func(data, reps, ndim):
+ out = output_tensor((ndim,), "int64")
+
+ for i in const_range(ndim):
+ out[i] = data.shape[i] * int64(reps[i])
+ return out
+
+
+@_reg.register_shape_func("dyn.tile", True)
+def tile_shape_func(attrs, inputs, _):
+ """
+ Shape function for tile op.
+ """
+ ndim = len(inputs[0].shape)
+ rdim = inputs[1].shape[0].value
+ assert ndim == rdim, "tile data and reps ranks don't match"
Review comment:
Yes, to prevent dynamic rank behavior, I matched the TF/ONNX APIs and only allow dynamic tile where the rank of the input and the size of the reps match. This is slightly different than the standard TOPI op which implicitly pads either the input shape or the reps with ones.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450343053
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -46,6 +47,14 @@ class DynamicToStaticMutator : public MixedModeMutator {
static const Op& reshape = Op::Get("reshape");
return Call(reshape, {call_node->args[0]}, Attrs(attrs), {});
}
+ } else if (call_node->op == dyn_tile_op_) {
Review comment:
Just out of curiosity, the dynamic list could be quite long, will we enumerate each case here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450403571
##########
File path: python/tvm/relay/op/dyn/_transform.py
##########
@@ -81,3 +84,23 @@ def _reshape_shape_func_input_data(data, newshape, ndim):
@_reg.register_shape_func("dyn.reshape", True)
def dynamic_reshape_shape_func(attrs, inputs, out_ndims):
return [_reshape_shape_func_input_data(*inputs, out_ndims[0])]
+
+
+@script
+def _tile_shape_func(data, reps, ndim):
+ out = output_tensor((ndim,), "int64")
+
+ for i in const_range(ndim):
+ out[i] = data.shape[i] * int64(reps[i])
+ return out
+
+
+@_reg.register_shape_func("dyn.tile", True)
+def tile_shape_func(attrs, inputs, _):
+ """
+ Shape function for tile op.
+ """
+ ndim = len(inputs[0].shape)
+ rdim = inputs[1].shape[0].value
+ assert ndim == rdim, "tile data and reps ranks don't match"
Review comment:
I've been looking at it again this morning, I'm trying to convince myself I might be able to re-implement the MXNet-like behavior without dynamic ranks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450485913
##########
File path: python/tvm/relay/op/dyn/_transform.py
##########
@@ -81,3 +84,23 @@ def _reshape_shape_func_input_data(data, newshape, ndim):
@_reg.register_shape_func("dyn.reshape", True)
def dynamic_reshape_shape_func(attrs, inputs, out_ndims):
return [_reshape_shape_func_input_data(*inputs, out_ndims[0])]
+
+
+@script
+def _tile_shape_func(data, reps, ndim):
+ out = output_tensor((ndim,), "int64")
+
+ for i in const_range(ndim):
+ out[i] = data.shape[i] * int64(reps[i])
+ return out
+
+
+@_reg.register_shape_func("dyn.tile", True)
+def tile_shape_func(attrs, inputs, _):
+ """
+ Shape function for tile op.
+ """
+ ndim = len(inputs[0].shape)
+ rdim = inputs[1].shape[0].value
+ assert ndim == rdim, "tile data and reps ranks don't match"
Review comment:
Cool. Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450353012
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -46,6 +47,14 @@ class DynamicToStaticMutator : public MixedModeMutator {
static const Op& reshape = Op::Get("reshape");
return Call(reshape, {call_node->args[0]}, Attrs(attrs), {});
}
+ } else if (call_node->op == dyn_tile_op_) {
Review comment:
I think we'll need to, there's no other way to dispatch on individual ops types. We could do it with a switch statement for performance, or implement a registry lookup to move the code out of of this function.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450524033
##########
File path: src/relay/op/dyn/tensor/transform.cc
##########
@@ -128,6 +130,71 @@ RELAY_REGISTER_OP("dyn.reshape")
.set_attr<FTVMCompute>("FTVMCompute", ReshapeCompute)
.set_attr<TOpPattern>("TOpPattern", kInjective);
+// tile operator
+// TVM_REGISTER_NODE_TYPE(TileAttrs);
Review comment:
Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450374695
##########
File path: src/relay/transforms/dynamic_to_static.cc
##########
@@ -46,6 +47,14 @@ class DynamicToStaticMutator : public MixedModeMutator {
static const Op& reshape = Op::Get("reshape");
return Call(reshape, {call_node->args[0]}, Attrs(attrs), {});
}
+ } else if (call_node->op == dyn_tile_op_) {
Review comment:
As the purely dynamic ops mature, I'm thinking we'll add this pass to the defaults so it runs a a normal part of compilation. Until then, without this pass dynamic ops will run on the VM and fail on the Graph Runtime, as we have with current behavior.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics merged pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
zhiics merged pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#issuecomment-655055241
Thanks @mbrookhart @siju-samuel @electriclilies @yongwww
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450483728
##########
File path: python/tvm/relay/op/dyn/_transform.py
##########
@@ -81,3 +84,23 @@ def _reshape_shape_func_input_data(data, newshape, ndim):
@_reg.register_shape_func("dyn.reshape", True)
def dynamic_reshape_shape_func(attrs, inputs, out_ndims):
return [_reshape_shape_func_input_data(*inputs, out_ndims[0])]
+
+
+@script
+def _tile_shape_func(data, reps, ndim):
+ out = output_tensor((ndim,), "int64")
+
+ for i in const_range(ndim):
+ out[i] = data.shape[i] * int64(reps[i])
+ return out
+
+
+@_reg.register_shape_func("dyn.tile", True)
+def tile_shape_func(attrs, inputs, _):
+ """
+ Shape function for tile op.
+ """
+ ndim = len(inputs[0].shape)
+ rdim = inputs[1].shape[0].value
+ assert ndim == rdim, "tile data and reps ranks don't match"
Review comment:
I think I got it to work. I wasn't able to use the infrastructure from the static case directly, but it now implements the same API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r450395718
##########
File path: python/tvm/relay/op/dyn/_transform.py
##########
@@ -81,3 +84,23 @@ def _reshape_shape_func_input_data(data, newshape, ndim):
@_reg.register_shape_func("dyn.reshape", True)
def dynamic_reshape_shape_func(attrs, inputs, out_ndims):
return [_reshape_shape_func_input_data(*inputs, out_ndims[0])]
+
+
+@script
+def _tile_shape_func(data, reps, ndim):
+ out = output_tensor((ndim,), "int64")
+
+ for i in const_range(ndim):
+ out[i] = data.shape[i] * int64(reps[i])
+ return out
+
+
+@_reg.register_shape_func("dyn.tile", True)
+def tile_shape_func(attrs, inputs, _):
+ """
+ Shape function for tile op.
+ """
+ ndim = len(inputs[0].shape)
+ rdim = inputs[1].shape[0].value
+ assert ndim == rdim, "tile data and reps ranks don't match"
Review comment:
Last question, I notice that the shape_func here is somehow different from the previous impl
https://github.com/apache/incubator-tvm/blob/151f3f5a00b5b2e2a369b9169dc8cda02e87e82a/python/tvm/relay/op/_transform.py#L609
is it expected?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] siju-samuel commented on a change in pull request #5983: Dynamic Tile Op
Posted by GitBox <gi...@apache.org>.
siju-samuel commented on a change in pull request #5983:
URL: https://github.com/apache/incubator-tvm/pull/5983#discussion_r449516436
##########
File path: python/tvm/relay/op/dyn/_transform.py
##########
@@ -81,3 +84,23 @@ def _reshape_shape_func_input_data(data, newshape, ndim):
@_reg.register_shape_func("dyn.reshape", True)
def dynamic_reshape_shape_func(attrs, inputs, out_ndims):
return [_reshape_shape_func_input_data(*inputs, out_ndims[0])]
+
+
+@script
+def _tile_shape_func(data, reps, ndim):
+ out = output_tensor((ndim,), "int64")
+
+ for i in const_range(ndim):
+ out[i] = data.shape[i] * int64(reps[i])
+ return out
+
+
+@_reg.register_shape_func("dyn.tile", True)
+def tile_shape_func(attrs, inputs, _):
+ """
+ Shape function for tile op.
+ """
+ ndim = len(inputs[0].shape)
+ rdim = inputs[1].shape[0].value
+ assert ndim == rdim, "tile data and res ranks don't match"
Review comment:
:s/res/reps
##########
File path: topi/include/topi/transform.h
##########
@@ -1016,6 +1016,38 @@ inline Tensor tile(const Tensor& x, Array<Integer> reps, std::string name = "T_t
}
}
+/*!
+ * \brief Creates an operation to tile elements of an array
+ *
+ * \param x The input tensor
+ * \param reps The number of times for repeating the tensor
+ * \param name The name of the operation
+ * \param tag The tag to mark the operation
+ *
+ * \return A Tensor whose op member is the tile operation
+ */
+inline Tensor dyn_tile(const Tensor& x, Array<PrimExpr> new_shape, std::string name = "T_tile",
+ std::string tag = kBroadcast) {
+ size_t ndim = x->shape.size();
+ std::cout << ndim << std::endl;
+ std::cout << new_shape << std::endl;
Review comment:
remove the couts
##########
File path: topi/include/topi/transform.h
##########
@@ -1016,6 +1016,38 @@ inline Tensor tile(const Tensor& x, Array<Integer> reps, std::string name = "T_t
}
}
+/*!
+ * \brief Creates an operation to tile elements of an array
+ *
+ * \param x The input tensor
+ * \param reps The number of times for repeating the tensor
+ * \param name The name of the operation
Review comment:
update the header
##########
File path: tests/python/relay/dyn/test_dynamic_op_level3.py
##########
@@ -70,6 +70,24 @@ def verify_reshape(shape, newshape, oshape):
verify_reshape((2, 3, 4), (8, 3), (8, 3))
verify_reshape((4, 7), (2, 7, 2), (2, 7, 2))
+def test_dyn_tile():
+ def verify_tile(dshape, reps):
+ x = relay.var("x", relay.TensorType(dshape, "float32"))
+ r = relay.var("reps", relay.TensorType((len(dshape), ), "float32"))
+ z = relay.tile(x, r)
+
+ func = relay.Function([x, r], z)
+ x_data = np.random.uniform(low=-1, high=1, size=dshape).astype("float32")
+ ref_res = np.tile(x_data, reps=reps)
+ print (ref_res.shape)
+ print(x_data.shape)
+ print(np.array(reps).shape)
Review comment:
remove prints
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org