[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
lixiaoquan commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r422950273
##
File path: python/tvm/relay/op/_transform.py
##
@@ -189,10 +192,9 @@ def _reshape_shape_func(data_shape, newshape, ndim):
out[infer_idx] = old_size // new_size
return out
-@_reg.register_shape_func("reshape", False)
+@_reg.register_shape_func("reshape", True)
Review comment:
@icemelon9 I've solved your concern. Could you please take another
look? 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:
us...@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
lixiaoquan commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r420541474
##
File path: tests/python/relay/test_any.py
##
@@ -138,23 +138,35 @@ def test_any_concat():
result = ex.evaluate()(x_np, y_np)
tvm.testing.assert_allclose(result.asnumpy(), ref)
-def verify_any_reshape(x_shape, newshape, x_np_shape, out_shape):
+def verify_any_reshape(x_shape, newshape, x_np_shape, out_shape,
variable_newshape=False):
x = relay.var('x', shape=x_shape, dtype="float32")
+data = np.random.uniform(size=x_np_shape).astype('float32')
+params = [x]
+args = [data]
+
+if variable_newshape:
+newshape_var = relay.var('newshape', shape=(len(newshape),),
dtype='int64')
+params.append(newshape_var)
+args.append(np.array(newshape, dtype='int64'))
+newshape = newshape_var
+
y = relay.reshape(x, newshape=newshape)
mod = tvm.IRModule()
-mod["main"] = relay.Function([x], y)
-data = np.random.uniform(size=x_np_shape).astype('float32')
+mod["main"] = relay.Function(params, y)
+
for kind in ["debug", "vm"]:
ex = relay.create_executor(kind, mod=mod, ctx=tvm.cpu(), target="llvm")
-result = ex.evaluate()(data).asnumpy()
+result = ex.evaluate()(*args).asnumpy()
assert result.shape == out_shape
tvm.testing.assert_allclose(result.flatten(), data.flatten())
def test_any_reshape():
-verify_any_reshape(any_dims(3), (1, -1), (2, 3, 4), (1, 24))
-verify_any_reshape(any_dims(3), (0, -1), (2, 3, 4), (2, 12))
+for variable_newshape in [False, True]:
+# Variable newshape only supports that output rank is the same as
newshape
Review comment:
Do you mean add some dynamic shape cases to test_op_level3? Cases in
that file use 'graph' exectuor now and test like test_arange() only supports
constant inputs too.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
lixiaoquan commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r420541295
##
File path: src/relay/transforms/pattern_util.h
##
@@ -270,7 +270,7 @@ inline Constant MakeConstantScalar(DataType dtype, T value)
{
*/
template
static inline Constant MakeConstantTensor(DataType dtype, std::vector
shape,
- std::vector value) {
+ T value) {
Review comment:
Sorry for late. Updated
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
lixiaoquan commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r418780844
##
File path: src/relay/transforms/fuse_ops.cc
##
@@ -234,6 +235,8 @@ class IndexedForwardGraph::Creator : private ExprVisitor {
Node* node = graph_.node_map.at(call);
static auto fpattern =
Op::GetAttr("TOpPattern");
+static auto tshape_dependant = Op::GetAttr(
+"TShapeDataDependant");
Review comment:
Yes, this is used to prevent a result of shape function from being fed
to a data dependent shape 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:
us...@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
lixiaoquan commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r418341379
##
File path: include/tvm/relay/op_attr_types.h
##
@@ -81,10 +81,16 @@ using TOpIsStateful = bool;
*/
using TNonComputational = bool;
+enum ShapeDependantKind {
+ kShapeDependantShape = 0,
+ kShapeDependantData = 1,
+ kShapeDependantBoth = 2,
Review comment:
@icemelon9 This is solved, could you please take another look?
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
lixiaoquan commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r415554589
##
File path: include/tvm/relay/op_attr_types.h
##
@@ -81,10 +81,16 @@ using TOpIsStateful = bool;
*/
using TNonComputational = bool;
+enum ShapeDependantKind {
+ kShapeDependantShape = 0,
+ kShapeDependantData = 1,
+ kShapeDependantBoth = 2,
Review comment:
Thanks, it's solved now
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
lixiaoquan commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r415261490
##
File path: include/tvm/relay/op_attr_types.h
##
@@ -81,10 +81,16 @@ using TOpIsStateful = bool;
*/
using TNonComputational = bool;
+enum ShapeDependantKind {
+ kShapeDependantShape = 0,
+ kShapeDependantData = 1,
+ kShapeDependantBoth = 2,
Review comment:
https://github.com/apache/incubator-tvm/blob/6e1cd8256e461deca51e45e7aae33088e8f5b966/python/tvm/relay/op/_transform.py#L179
This line uses a range() and I think it causes variable index.
I tried to allocate a data shape tensor by tvm.te.placeholder(), but it
can't be passed to tvm.lower() when this shape func is lowered.
But data shape tensor allocated in this line will be stored in CachedNode
object and sent to tvm.lower(), that's why I have to pass data shape tensor
(https://github.com/apache/incubator-tvm/blob/2c1ca60ea38587401a20f11c5e64f452b79fa777/src/relay/backend/compile_engine.cc#L348)
Please correct me if I missunderstood your suggestion about allocating a
buffer.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
lixiaoquan commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r415261490
##
File path: include/tvm/relay/op_attr_types.h
##
@@ -81,10 +81,16 @@ using TOpIsStateful = bool;
*/
using TNonComputational = bool;
+enum ShapeDependantKind {
+ kShapeDependantShape = 0,
+ kShapeDependantData = 1,
+ kShapeDependantBoth = 2,
Review comment:
https://github.com/apache/incubator-tvm/blob/6e1cd8256e461deca51e45e7aae33088e8f5b966/python/tvm/relay/op/_transform.py#L179
This line uses a range() and I think it causes variable index.
I tried to allocate a data shape tensor by tvm.te.placeholder(), but it
can't be passed to tvm.lower() when this shape func is lowered.
But data shape tensor allocated in this line will be stored in CachedNode
object
(https://github.com/apache/incubator-tvm/blob/2c1ca60ea38587401a20f11c5e64f452b79fa777/src/relay/backend/compile_engine.cc#L348)
Please correct me if I missunderstood your suggestion about allocating a
buffer.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
lixiaoquan commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r415197766
##
File path: include/tvm/relay/op_attr_types.h
##
@@ -81,10 +81,16 @@ using TOpIsStateful = bool;
*/
using TNonComputational = bool;
+enum ShapeDependantKind {
+ kShapeDependantShape = 0,
+ kShapeDependantData = 1,
+ kShapeDependantBoth = 2,
Review comment:
Thanks for you comment.
I tried two ways to use data tensor's shape directly.
First, I passed data tensor's shape to _reshape_shape_func but I got this
error
```
ValueError: All indices are supposed to be constants
```
Second, I tried to construct a tensor by tvm.te.placeholder based on data
tensor's shape, but it doesn't work because it can't be passed to tvm.lower
called later.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
lixiaoquan commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r415196802
##
File path: include/tvm/relay/op_attr_types.h
##
@@ -81,10 +81,16 @@ using TOpIsStateful = bool;
*/
using TNonComputational = bool;
+enum ShapeDependantKind {
+ kShapeDependantShape = 0,
+ kShapeDependantData = 1,
+ kShapeDependantBoth = 2,
Review comment:
Thanks for your comment
I checked #4312, and tried to use data.shape the same ways as
_strided_slice_shape_func(), but I got this error from hybrid script parser
```
ValueError: All indices are supposed to be constants
```
it seems _strided_slice_shape_func only uses const index when accessing
data.shape, but _reshape_shape_func uses variables as indices to access
data_shape
I'd appreciate it if you can give me some hint about how to solve this.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] lixiaoquan commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
lixiaoquan commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r415196802
##
File path: include/tvm/relay/op_attr_types.h
##
@@ -81,10 +81,16 @@ using TOpIsStateful = bool;
*/
using TNonComputational = bool;
+enum ShapeDependantKind {
+ kShapeDependantShape = 0,
+ kShapeDependantData = 1,
+ kShapeDependantBoth = 2,
Review comment:
I checked #4312, and tried to use data.shape the same ways as
_strided_slice_shape_func(), but I got this error from hybrid script parser
```
ValueError: All indices are supposed to be constants
```
it seems _strided_slice_shape_func only uses const index when accessing
data.shape, but _reshape_shape_func uses variables as indices to access
data_shape
I'd appreciate it if you can give me some hint about how to solve this.
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:
us...@infra.apache.org
