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:
[email protected]
