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/09 03:51:35 UTC
[GitHub] [incubator-tvm] Haoran-Young opened a new pull request #6022: Add support for checking big ONNX models
Haoran-Young opened a new pull request #6022:
URL: https://github.com/apache/incubator-tvm/pull/6022
Some users adopt ONNX to convert and save their trained models. But some of the models are big, which have sizes larger than 2GB. In these cases, ONNX cannot save these big models successfully. Recently, developers of ONNX have made some updates on ONNX so that ONNX can save, load, check big models.
I note that TVM cannot yet support compiling big ONNX models because TVM fails to check big ONNX models. I have made minor modifications to the source codes so that TVM can support checking and compiling big ONNX models.
@adityaatluri @liangfu @zhiics
Could you please have a look on this 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] adityaatluri commented on pull request #6022: Add support for checking big ONNX models
Posted by GitBox <gi...@apache.org>.
adityaatluri commented on pull request #6022:
URL: https://github.com/apache/incubator-tvm/pull/6022#issuecomment-656425163
Can the API detect the big vs small model without a function argument?
----------------------------------------------------------------
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] Haoran-Young commented on pull request #6022: Add support for checking big ONNX models
Posted by GitBox <gi...@apache.org>.
Haoran-Young commented on pull request #6022:
URL: https://github.com/apache/incubator-tvm/pull/6022#issuecomment-656444288
If you do not want to use additional argument to verify whether a model is big or small, I will replace the argument 'model' with 'model_path'. In this case, no model will be imported into the function 'from_onnx' and I can utilize the ONNX API to verify whether the model is big or small without any argument except the model path.
Further, if the big model and its external data files (external data files are generated during saving big ONNX model) are not under the same dir, there must be an additional argument to point out the path of external data files.
----------------------------------------------------------------
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 closed pull request #6022: Add support for checking big ONNX models
Posted by GitBox <gi...@apache.org>.
tqchen closed pull request #6022:
URL: https://github.com/apache/incubator-tvm/pull/6022
----------------------------------------------------------------
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] liangfu commented on pull request #6022: Add support for checking big ONNX models
Posted by GitBox <gi...@apache.org>.
liangfu commented on pull request #6022:
URL: https://github.com/apache/incubator-tvm/pull/6022#issuecomment-664790283
ping @Haoran-Young , please follow-up on @jwfromm 's comments.
----------------------------------------------------------------
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 #6022: Add support for checking big ONNX models
Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #6022:
URL: https://github.com/apache/incubator-tvm/pull/6022#issuecomment-655899793
cc @jwfromm, please help review. 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] jwfromm commented on a change in pull request #6022: Add support for checking big ONNX models
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #6022:
URL: https://github.com/apache/incubator-tvm/pull/6022#discussion_r453773341
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -2254,8 +2255,13 @@ def from_onnx(model,
Parameters
----------
- model : protobuf object
- ONNX ModelProto after ONNX v1.1.0
+ model_path : str
+ The path of the ONNX model
Review comment:
I recommend allowing the `model` argument to be either an Onnx ModelProto or a string pointing to the saved Onnx file. You can then add a check to see if its a string and do `onnx.load` and `load_external_data_for_model`. Otherwise parse the OnnxProto as normal. This will make it a lot easier to deal with the existing TVM codebase and allow your tests to pass.
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -2239,7 +2239,8 @@ def _fix_outputs(self, op_name, outputs):
outputs = outputs[:-1]
return outputs
-def from_onnx(model,
+def from_onnx(model_path=None,
+ external_data_files_path=None,
Review comment:
Put `external_data_files_path` further down (ideally last) in the argument list. All the importers have the first two arguments as model, shape_dict and it'd be nice to keep this consistent.
----------------------------------------------------------------
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] liangfu commented on a change in pull request #6022: Add support for checking big ONNX models
Posted by GitBox <gi...@apache.org>.
liangfu commented on a change in pull request #6022:
URL: https://github.com/apache/incubator-tvm/pull/6022#discussion_r452591242
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -2263,6 +2265,14 @@ def from_onnx(model,
dtype : str or dict of str to str
The input types to the graph
+ big_model : bool
+ Whether the size of the input model is larger than 2GB
+
+ model_path : str, optional
+ The path of input model, and the external data
+ files need under the same directory
+ Must be set if 'big_model' is True
Review comment:
In this case, I think it's better to check whether model_path is None, instead of providing an additional big_model argument.
----------------------------------------------------------------
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] Haoran-Young commented on a change in pull request #6022: Add support for checking big ONNX models
Posted by GitBox <gi...@apache.org>.
Haoran-Young commented on a change in pull request #6022:
URL: https://github.com/apache/incubator-tvm/pull/6022#discussion_r452643452
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -2263,6 +2265,14 @@ def from_onnx(model,
dtype : str or dict of str to str
The input types to the graph
+ big_model : bool
+ Whether the size of the input model is larger than 2GB
+
+ model_path : str, optional
+ The path of input model, and the external data
+ files need under the same directory
+ Must be set if 'big_model' is True
Review comment:
@liangfu Thanks for your advice.
As I commented on @adityaatluri 's advice, I think it is better to introduce two arguments, 'model_path' and 'external_data_path,' to replace 'model,' 'big_model,' 'model_path.' in my commit.
The newest ONNX API can load a model via model path and external data files' path (external files' path is needed if they are not under the same directory with the model file) without any other arguments.
----------------------------------------------------------------
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