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/06/16 18:59:34 UTC

[GitHub] [incubator-tvm] mbrookhart opened a new pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

mbrookhart opened a new pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826


   Initial POC for ideas discussed in https://discuss.tvm.ai/t/dynamic-ops-in-relay/6909
   
   @tqchen @jroesch @kevinthesun @lixiaoquan @icemelon9 


----------------------------------------------------------------
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] electriclilies commented on a change in pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
electriclilies commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r444465147



##########
File path: python/tvm/relay/op/dyn/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):
+    """Reshape the input array based on the values of the newshape tensor.
+
+    To give user more convenience in without doing manual shape inference,
+    some dimensions of the shape can take special values from the set {0, -1, -3}.
+    The significance of each is explained below:
+
+    ``0`` copy this dimension from the input to the output shape.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (4,0,2), result.shape = (4,3,2)
+            data.shape = (2,3,4), newshape = (2,0,0), result.shape = (2,3,4)
+
+    ``-1`` infers the dimension of the output shape by using the remainder of
+    the input dimensions keeping the size of the new array same as that of the input array.
+    At most one dimension of shape can be -1.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (6,1,-1), result.shape = (6,1,4)
+            data.shape = (2,3,4), newshape = (3,-1,8), result.shape = (3,1,8)
+            data.shape = (2,3,4), newshape = (-1,), result.shape = (24,)
+
+    ``-3`` use the product of two consecutive dimensions of the input shape
+    as the output dimension.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (-3,4), result.shape = (6,4)
+            data.shape = (2,3,4,5), newshape = (-3,-3), result.shape = (6,20)
+            data.shape = (2,3,4), newshape = (0,-3), result.shape = (2,12)
+            data.shape = (2,3,4), newshape = (-3,-2), result.shape = (6,4)

Review comment:
       This example uses -2 as an argument (in the last example), but it was noted earlier that -2 is not allowed as an argument in dynamic reshape. 




----------------------------------------------------------------
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] icemelon9 merged pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
icemelon9 merged pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826


   


----------------------------------------------------------------
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 pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#issuecomment-650348127


   @icemelon9 @lixiaoquan Per Haichen's request, I removed the dynamic option from the standard reshape and changed the places that used it to use the new purely dynamic op. This touches many more files, could you 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:
users@infra.apache.org



[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r444322275



##########
File path: python/tvm/relay/op/dyn/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):

Review comment:
       This is an interesting idea. It makes buy into the dynamic op namespace implicit, which reduces changes needed in the frontends later, but it also forces people to use the namespace earlier. @jroesch What do you think, is implicit or explicit better at this stage?




----------------------------------------------------------------
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] icemelon9 commented on pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
icemelon9 commented on pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#issuecomment-649765936


   @mbrookhart Since we separate the static and dynamic reshape in this pr, we should remove the `Optional` in the `ReshapeAttrs` correspondingly.


----------------------------------------------------------------
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] jroesch commented on pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#issuecomment-647854063


   @icemelon9 That was my initial suggestion, I think we reached consensus between TQ, Matt and I that we would introduce dynamic and slowly migrate stuff to use them, then introduce static migrate everything to that, and then finally remove the dynamic prefix inorder to ensure a smooth migration without confusion. 


----------------------------------------------------------------
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] icemelon9 commented on pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
icemelon9 commented on pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#issuecomment-646810997


   Shall we just call the namespace `dyn`? I'm okay with `dynamic`, but just feel it is a bit long.


----------------------------------------------------------------
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 pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#issuecomment-647008054


   :+1: Happy to shorten it.


----------------------------------------------------------------
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] tqchen commented on pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#issuecomment-649044252


   cc @kevinthesun @lixiaoquan given that we agreed on the new convention, it would be great if we can also migrate some of the existing dynamic(symbolic) effort towards this direction


----------------------------------------------------------------
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] icemelon9 commented on a change in pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
icemelon9 commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r448548105



##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -511,7 +511,7 @@ def batch_matmul_grad(orig, grad):
 @register_gradient("reshape")
 def reshape_grad(orig, grad):
     """Gradient of reshape"""
-    return [reshape_like(grad, orig.args[0]), orig.args[1]]
+    return [reshape_like(grad, orig.args[0])]

Review comment:
       Agree.




----------------------------------------------------------------
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 #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r447056086



##########
File path: python/tvm/relay/op/dyn/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):

Review comment:
       Okay, I'll rework the python API to merge them.




----------------------------------------------------------------
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] tqchen edited a comment on pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#issuecomment-649616499


   cc @icemelon9 please followup and manage this PR, @jroesch @electriclilies @lixiaoquan  please https://tvm.apache.org/docs/contribute/code_review.html#approve-and-request-changes-explicitly


----------------------------------------------------------------
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] icemelon9 commented on a change in pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
icemelon9 commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r443892685



##########
File path: python/tvm/relay/op/dyn/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):

Review comment:
       I suggest in the frontend we only keep one `reshape` and inside the frontend you can decide whether to use `dyn.reshape` or `reshape`. So this means that we remove this file and instead update the `reshape` in the op/transform.py




----------------------------------------------------------------
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] jroesch commented on a change in pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
jroesch commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r446388132



##########
File path: python/tvm/relay/op/dyn/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):

Review comment:
       I think its okay to have a single interface in the Python code, at least I talked with Haichen about it offline. It seems less confusing for end users who want to build models, but will require that people understand the difference between the two ops in the backend (which is true either way).




----------------------------------------------------------------
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 pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#issuecomment-646347835


   Hitting an issue with dynamic shapes on GPU with the VM.


----------------------------------------------------------------
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] icemelon9 commented on a change in pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
icemelon9 commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r448021317



##########
File path: src/relay/analysis/util.cc
##########
@@ -448,13 +448,8 @@ bool IsDataDependant(const CallNode* call) {
     return false;
   }
 
-  if (op->name == "reshape") {
-    if (const auto* attrs = call->attrs.as<ReshapeAttrs>()) {
-      if (attrs->newshape) {
-        // If newshape attribute exists, it isn't data dependant.
-        return false;
-      }
-    }
+  if (op->name == "dyn.reshape") {
+    return true;

Review comment:
       We can remove line 451-452 as the shape function of `dyn.reshape` is registered to be data dependent.

##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -511,7 +511,7 @@ def batch_matmul_grad(orig, grad):
 @register_gradient("reshape")
 def reshape_grad(orig, grad):
     """Gradient of reshape"""
-    return [reshape_like(grad, orig.args[0]), orig.args[1]]
+    return [reshape_like(grad, orig.args[0])]

Review comment:
       Do we need to register a gradient op to `dyn.reshape`?

##########
File path: src/relay/op/tensor/transform.h
##########
@@ -38,7 +38,7 @@
 namespace tvm {
 namespace relay {
 
-extern Expr MakeReshape(Expr data, Expr newshape);
+extern Expr MakeReshape(Expr data, Array<Integer> newshape);

Review comment:
       ```suggestion
   Expr MakeReshape(Expr data, Array<Integer> newshape);
   ```




----------------------------------------------------------------
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] lixiaoquan commented on a change in pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
lixiaoquan commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r442871822



##########
File path: python/tvm/relay/op/dynamic/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):
+    """Reshape the input array based on the values of the newshape tensor.
+
+    To give user more convenience in without doing manual shape inference,
+    some dimensions of the shape can take special values from the set {0, -1, -3}.
+    The significance of each is explained below:
+
+    ``0`` copy this dimension from the input to the output shape.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (4,0,2), result.shape = (4,3,2)
+            data.shape = (2,3,4), newshape = (2,0,0), result.shape = (2,3,4)
+
+    ``-1`` infers the dimension of the output shape by using the remainder of
+    the input dimensions keeping the size of the new array same as that of the input array.
+    At most one dimension of shape can be -1.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (6,1,-1), result.shape = (6,1,4)
+            data.shape = (2,3,4), newshape = (3,-1,8), result.shape = (3,1,8)
+            data.shape = (2,3,4), newshape = (-1,), result.shape = (24,)
+
+    ``-3`` use the product of two consecutive dimensions of the input shape
+    as the output dimension.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (-3,4), result.shape = (6,4)
+            data.shape = (2,3,4,5), newshape = (-3,-3), result.shape = (6,20)
+            data.shape = (2,3,4), newshape = (0,-3), result.shape = (2,12)
+            data.shape = (2,3,4), newshape = (-3,-2), result.shape = (6,4)
+
+    Special values -2 and -4 from the standard reshape op would introduce dynamic rank
+    in this op. Thus, they are not permitted.

Review comment:
       Current code allows -2 and -4 as constant value. Should frontend check it like
   ```
   if newshape_inferable() and newshape == (-2) or newshape == (-4):
       return _op.reshape()
   else:
       return _op.dynamic.reshape()
   ```

##########
File path: python/tvm/relay/op/dynamic/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):
+    """Reshape the input array based on the values of the newshape tensor.
+
+    To give user more convenience in without doing manual shape inference,
+    some dimensions of the shape can take special values from the set {0, -1, -3}.
+    The significance of each is explained below:
+
+    ``0`` copy this dimension from the input to the output shape.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (4,0,2), result.shape = (4,3,2)
+            data.shape = (2,3,4), newshape = (2,0,0), result.shape = (2,3,4)
+
+    ``-1`` infers the dimension of the output shape by using the remainder of
+    the input dimensions keeping the size of the new array same as that of the input array.
+    At most one dimension of shape can be -1.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (6,1,-1), result.shape = (6,1,4)
+            data.shape = (2,3,4), newshape = (3,-1,8), result.shape = (3,1,8)
+            data.shape = (2,3,4), newshape = (-1,), result.shape = (24,)
+
+    ``-3`` use the product of two consecutive dimensions of the input shape
+    as the output dimension.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (-3,4), result.shape = (6,4)
+            data.shape = (2,3,4,5), newshape = (-3,-3), result.shape = (6,20)
+            data.shape = (2,3,4), newshape = (0,-3), result.shape = (2,12)
+            data.shape = (2,3,4), newshape = (-3,-2), result.shape = (6,4)
+
+    Special values -2 and -4 from the standard reshape op would introduce dynamic rank
+    in this op. Thus, they are not permitted.

Review comment:
       Current code allows -2 and -4 as constant value. Should frontend checks it like
   ```
   if newshape_inferable() and newshape == (-2) or newshape == (-4):
       return _op.reshape()
   else:
       return _op.dynamic.reshape()
   ```




----------------------------------------------------------------
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] tqchen commented on pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#issuecomment-646857875


   I agree with @icemelon9 perhaps `dyn` is a better name 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 #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r447103476



##########
File path: python/tvm/relay/op/dyn/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):

Review comment:
       I merged the two interfaces, let me know what you think.




----------------------------------------------------------------
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] electriclilies commented on a change in pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
electriclilies commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r444465147



##########
File path: python/tvm/relay/op/dyn/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):
+    """Reshape the input array based on the values of the newshape tensor.
+
+    To give user more convenience in without doing manual shape inference,
+    some dimensions of the shape can take special values from the set {0, -1, -3}.
+    The significance of each is explained below:
+
+    ``0`` copy this dimension from the input to the output shape.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (4,0,2), result.shape = (4,3,2)
+            data.shape = (2,3,4), newshape = (2,0,0), result.shape = (2,3,4)
+
+    ``-1`` infers the dimension of the output shape by using the remainder of
+    the input dimensions keeping the size of the new array same as that of the input array.
+    At most one dimension of shape can be -1.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (6,1,-1), result.shape = (6,1,4)
+            data.shape = (2,3,4), newshape = (3,-1,8), result.shape = (3,1,8)
+            data.shape = (2,3,4), newshape = (-1,), result.shape = (24,)
+
+    ``-3`` use the product of two consecutive dimensions of the input shape
+    as the output dimension.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (-3,4), result.shape = (6,4)
+            data.shape = (2,3,4,5), newshape = (-3,-3), result.shape = (6,20)
+            data.shape = (2,3,4), newshape = (0,-3), result.shape = (2,12)
+            data.shape = (2,3,4), newshape = (-3,-2), result.shape = (6,4)

Review comment:
       This example uses -2 as an argument (in the last example), but -2 is not allowed as an argument in dynamic reshape. 




----------------------------------------------------------------
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] icemelon9 commented on a change in pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
icemelon9 commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r445779288



##########
File path: python/tvm/relay/op/dyn/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):

Review comment:
       @jroesch Could you comment on 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:
users@infra.apache.org



[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r444505122



##########
File path: python/tvm/relay/op/dyn/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):
+    """Reshape the input array based on the values of the newshape tensor.
+
+    To give user more convenience in without doing manual shape inference,
+    some dimensions of the shape can take special values from the set {0, -1, -3}.
+    The significance of each is explained below:
+
+    ``0`` copy this dimension from the input to the output shape.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (4,0,2), result.shape = (4,3,2)
+            data.shape = (2,3,4), newshape = (2,0,0), result.shape = (2,3,4)
+
+    ``-1`` infers the dimension of the output shape by using the remainder of
+    the input dimensions keeping the size of the new array same as that of the input array.
+    At most one dimension of shape can be -1.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (6,1,-1), result.shape = (6,1,4)
+            data.shape = (2,3,4), newshape = (3,-1,8), result.shape = (3,1,8)
+            data.shape = (2,3,4), newshape = (-1,), result.shape = (24,)
+
+    ``-3`` use the product of two consecutive dimensions of the input shape
+    as the output dimension.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (-3,4), result.shape = (6,4)
+            data.shape = (2,3,4,5), newshape = (-3,-3), result.shape = (6,20)
+            data.shape = (2,3,4), newshape = (0,-3), result.shape = (2,12)
+            data.shape = (2,3,4), newshape = (-3,-2), result.shape = (6,4)

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] icemelon9 commented on pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
icemelon9 commented on pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#issuecomment-647842890


   In fact, I have another thought on this. Should we make the dynamic ops as the default ones whereas having static ops as special cases for better optimization?


----------------------------------------------------------------
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 #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r448039240



##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -511,7 +511,7 @@ def batch_matmul_grad(orig, grad):
 @register_gradient("reshape")
 def reshape_grad(orig, grad):
     """Gradient of reshape"""
-    return [reshape_like(grad, orig.args[0]), orig.args[1]]
+    return [reshape_like(grad, orig.args[0])]

Review comment:
       The numerical approx_grad doesn't seem to work for this case, and it doesn't seem the dynamic version was tested before. I need to look at the autodiff infrastructure more closely, perhaps this should be a second PR?




----------------------------------------------------------------
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 #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r442975459



##########
File path: python/tvm/relay/op/dynamic/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):
+    """Reshape the input array based on the values of the newshape tensor.
+
+    To give user more convenience in without doing manual shape inference,
+    some dimensions of the shape can take special values from the set {0, -1, -3}.
+    The significance of each is explained below:
+
+    ``0`` copy this dimension from the input to the output shape.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (4,0,2), result.shape = (4,3,2)
+            data.shape = (2,3,4), newshape = (2,0,0), result.shape = (2,3,4)
+
+    ``-1`` infers the dimension of the output shape by using the remainder of
+    the input dimensions keeping the size of the new array same as that of the input array.
+    At most one dimension of shape can be -1.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (6,1,-1), result.shape = (6,1,4)
+            data.shape = (2,3,4), newshape = (3,-1,8), result.shape = (3,1,8)
+            data.shape = (2,3,4), newshape = (-1,), result.shape = (24,)
+
+    ``-3`` use the product of two consecutive dimensions of the input shape
+    as the output dimension.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (-3,4), result.shape = (6,4)
+            data.shape = (2,3,4,5), newshape = (-3,-3), result.shape = (6,20)
+            data.shape = (2,3,4), newshape = (0,-3), result.shape = (2,12)
+            data.shape = (2,3,4), newshape = (-3,-2), result.shape = (6,4)
+
+    Special values -2 and -4 from the standard reshape op would introduce dynamic rank
+    in this op. Thus, they are not permitted.

Review comment:
       It's a runtime Expr, I can't check it at compile time.

##########
File path: python/tvm/relay/op/dynamic/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):
+    """Reshape the input array based on the values of the newshape tensor.
+
+    To give user more convenience in without doing manual shape inference,
+    some dimensions of the shape can take special values from the set {0, -1, -3}.
+    The significance of each is explained below:
+
+    ``0`` copy this dimension from the input to the output shape.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (4,0,2), result.shape = (4,3,2)
+            data.shape = (2,3,4), newshape = (2,0,0), result.shape = (2,3,4)
+
+    ``-1`` infers the dimension of the output shape by using the remainder of
+    the input dimensions keeping the size of the new array same as that of the input array.
+    At most one dimension of shape can be -1.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (6,1,-1), result.shape = (6,1,4)
+            data.shape = (2,3,4), newshape = (3,-1,8), result.shape = (3,1,8)
+            data.shape = (2,3,4), newshape = (-1,), result.shape = (24,)
+
+    ``-3`` use the product of two consecutive dimensions of the input shape
+    as the output dimension.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (-3,4), result.shape = (6,4)
+            data.shape = (2,3,4,5), newshape = (-3,-3), result.shape = (6,20)
+            data.shape = (2,3,4), newshape = (0,-3), result.shape = (2,12)
+            data.shape = (2,3,4), newshape = (-3,-2), result.shape = (6,4)
+
+    Special values -2 and -4 from the standard reshape op would introduce dynamic rank
+    in this op. Thus, they are not permitted.

Review comment:
       The Input is a dynamic Expression, I can't check it at graph construction time.




----------------------------------------------------------------
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 #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r442975459



##########
File path: python/tvm/relay/op/dynamic/transform.py
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel
+"""Dynamic Transform operators."""
+
+from . import _make
+
+
+def reshape(data, newshape):
+    """Reshape the input array based on the values of the newshape tensor.
+
+    To give user more convenience in without doing manual shape inference,
+    some dimensions of the shape can take special values from the set {0, -1, -3}.
+    The significance of each is explained below:
+
+    ``0`` copy this dimension from the input to the output shape.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (4,0,2), result.shape = (4,3,2)
+            data.shape = (2,3,4), newshape = (2,0,0), result.shape = (2,3,4)
+
+    ``-1`` infers the dimension of the output shape by using the remainder of
+    the input dimensions keeping the size of the new array same as that of the input array.
+    At most one dimension of shape can be -1.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (6,1,-1), result.shape = (6,1,4)
+            data.shape = (2,3,4), newshape = (3,-1,8), result.shape = (3,1,8)
+            data.shape = (2,3,4), newshape = (-1,), result.shape = (24,)
+
+    ``-3`` use the product of two consecutive dimensions of the input shape
+    as the output dimension.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (-3,4), result.shape = (6,4)
+            data.shape = (2,3,4,5), newshape = (-3,-3), result.shape = (6,20)
+            data.shape = (2,3,4), newshape = (0,-3), result.shape = (2,12)
+            data.shape = (2,3,4), newshape = (-3,-2), result.shape = (6,4)
+
+    Special values -2 and -4 from the standard reshape op would introduce dynamic rank
+    in this op. Thus, they are not permitted.

Review comment:
       It's a runtime Expr, I can't check it at compile time.




----------------------------------------------------------------
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] electriclilies commented on a change in pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
electriclilies commented on a change in pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#discussion_r444467091



##########
File path: src/relay/op/dyn/tensor/transform.cc
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file transform.cc
+ * \brief Dynamic Transform operators.
+ */
+#include "transform.h"
+
+#include <topi/transform.h>
+#include <tvm/relay/attrs/transform.h>
+#include <tvm/relay/op.h>
+#include <tvm/relay/op_attr_types.h>
+#include <tvm/runtime/registry.h>
+
+namespace tvm {
+namespace relay {
+namespace dyn {
+
+/* relay.reshape */
+// TVM_REGISTER_NODE_TYPE(ReshapeAttrs);
+
+bool ReshapeRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                const TypeReporter& reporter) {
+  // types: [data, newshape, result]
+  CHECK_EQ(types.size(), 3);
+
+  const auto* data = types[0].as<TensorTypeNode>();
+  if (data == nullptr) {
+    CHECK(types[0].as<IncompleteTypeNode>())
+        << "reshape: expect input type to be TensorType but get " << types[0];
+    return false;
+  }
+
+  Array<IndexExpr> oshape;
+  const auto* newshape = types[1].as<TensorTypeNode>();
+
+  // Doesn't support dynamic output rank
+  for (int i = 0; i < newshape->shape[0].as<IntImmNode>()->value; i++) {
+    oshape.push_back(Any());
+  }
+
+  reporter->Assign(types[2], TensorType(oshape, data->dtype));
+  return true;
+}
+
+Array<te::Tensor> ReshapeCompute(const Attrs& attrs, const Array<te::Tensor>& inputs,
+                                 const Type& out_type) {
+  const auto* out_ttype = out_type.as<TensorTypeNode>();
+  CHECK(out_ttype != nullptr);
+  Array<IndexExpr> newshape;
+  for (auto val : out_ttype->shape) {
+    if (val->IsInstance<tir::AnyNode>()) {
+      newshape.push_back(val.as<tir::AnyNode>()->ToVar());
+    } else {
+      newshape.push_back(val);
+    }
+  }
+  return {topi::reshape(inputs[0], newshape)};
+}
+
+Expr MakeReshape(Expr data, Expr newshape) {
+  auto attrs = make_object<ReshapeAttrs>();
+  attrs->reverse = false;
+  static const Op& op = Op::Get("dyn.reshape");
+  return Call(op, {data, newshape}, Attrs(attrs), {});
+}
+
+TVM_REGISTER_GLOBAL("relay.op.dyn._make.reshape").set_body_typed(MakeReshape);
+
+RELAY_REGISTER_OP("dyn.reshape")
+    .describe(R"code(Reshapes the input array based on the values in the newshape array.
+    
+    To give user more convenience in without doing manual shape inference,
+    some dimensions of the shape can take special values from the set {0, -1, -3}.
+    The significance of each is explained below:
+
+    ``0`` copy this dimension from the input to the output shape.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (4,0,2), result.shape = (4,3,2)
+            data.shape = (2,3,4), newshape = (2,0,0), result.shape = (2,3,4)
+
+    ``-1`` infers the dimension of the output shape by using the remainder of
+    the input dimensions keeping the size of the new array same as that of the input array.
+    At most one dimension of shape can be -1.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (6,1,-1), result.shape = (6,1,4)
+            data.shape = (2,3,4), newshape = (3,-1,8), result.shape = (3,1,8)
+            data.shape = (2,3,4), newshape = (-1,), result.shape = (24,)
+
+    ``-3`` use the product of two consecutive dimensions of the input shape
+    as the output dimension.
+
+        .. code-block:: python
+
+            data.shape = (2,3,4), newshape = (-3,4), result.shape = (6,4)
+            data.shape = (2,3,4,5), newshape = (-3,-3), result.shape = (6,20)
+            data.shape = (2,3,4), newshape = (0,-3), result.shape = (2,12)
+            data.shape = (2,3,4), newshape = (-3,-2), result.shape = (6,4)

Review comment:
       -2 is used as an argument in this example but -2 isn't allowed as an argument for dynamic reshape




----------------------------------------------------------------
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] tqchen commented on pull request #5826: [DYNAMIC] Add Dynamic reshape to a dynamic namespace and add DynamicToStatic Pass

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5826:
URL: https://github.com/apache/incubator-tvm/pull/5826#issuecomment-649616499


   cc @icemelon9 please followup and manage this PR, @jroesch @electriclilies please https://tvm.apache.org/docs/contribute/code_review.html#approve-and-request-changes-explicitly


----------------------------------------------------------------
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