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 2021/02/09 19:45:19 UTC

[GitHub] [tvm] mbrookhart opened a new pull request #7429: [WIP][ONNX] Make the ONNX Importer More Static

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


   We've been hitting a lot of issues in more complicated ONNX models where results of something like dynamic strided slice feed into an op like dynamic reshape, causing dynamic ranks to appear and crash the importer. To fix this, I decided it might be good to try to progagate constant information through the importer during import instead of trying to retroactively remove dynamic ops in a pass. To do that, I changed the freeze_params behavior of the importer to create constants before import, and made the importer fold constants through the importer as they are seen. Additionally, I edited the ops to only construct dynamic ops if the inputs are not constant.
   
   In sum, this ends up creating many fewer dynamic ops in the ONNX importer, and so we see many fewer dynamic rank issues when importing models.
   
   I've labeled this WIP for now, it depends on #7423 and I have a hack in place while waiting on that PR, but I'll like to get the full CI running and some eyes on it in the mean time.
   
   cc @masahi @jwfromm @electriclilies 
   
   


----------------------------------------------------------------
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] [tvm] electriclilies commented on a change in pull request #7429: [WIP][ONNX] Make the ONNX Importer More Static

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



##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -2850,12 +2879,16 @@ def from_onnx(self, graph, opset, freeze_params=False, get_output_expr=False):
         for init_tensor in graph.initializer:
             if not init_tensor.name.strip():
                 raise ValueError("Tensor's name is required.")
-            self._params[init_tensor.name] = self._parse_array(init_tensor)
-            self._nodes[init_tensor.name] = new_var(
-                init_tensor.name,
-                shape=self._params[init_tensor.name].shape,
-                dtype=self._params[init_tensor.name].dtype,
-            )
+            if freeze_params:
+                array = self._parse_array(init_tensor)
+                self._nodes[init_tensor.name] = _expr.const(array)
+            else:
+                self._params[init_tensor.name] = self._parse_array(init_tensor)

Review comment:
       You could pull `array = self._parse_array(init_tensor)` out of the if / else statement




----------------------------------------------------------------
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] [tvm] tmoreau89 merged pull request #7429: [ONNX] Make the ONNX Importer More Static

Posted by GitBox <gi...@apache.org>.
tmoreau89 merged pull request #7429:
URL: https://github.com/apache/tvm/pull/7429


   


----------------------------------------------------------------
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] [tvm] tmoreau89 commented on pull request #7429: [ONNX] Make the ONNX Importer More Static

Posted by GitBox <gi...@apache.org>.
tmoreau89 commented on pull request #7429:
URL: https://github.com/apache/tvm/pull/7429#issuecomment-778560234


   Thank you @mbrookhart @electriclilies @junrushao1994 the PR has been merged


----------------------------------------------------------------
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] [tvm] mbrookhart commented on pull request #7429: [ONNX] Make the ONNX Importer More Static

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


   @masahi @jwfromm @tmoreau89 I think this is ready for review
   


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