reminisce commented on issue #14040: Reformat of TensorRT to use subgraph API URL: https://github.com/apache/incubator-mxnet/pull/14040#issuecomment-461280671 @Caenorst Just want to clarify, I'm not blocking this PR. We can think through about these comments and make incremental changes afterwards. The point is adding zero overhead in terms of user experience of using TensorRT in MXNet. I don't think you need to pass the param data explicitly beforehand. They can all be collected at the first forward call for each subgraph. You can take a look how this is done in [TVM](https://github.com/reminisce/tvm/blob/subgraph_integration/src/contrib/subgraph/tensorrt_executor.cc#L918). On the high level, in the forward function of a tensorrt subgraph op, you have a list of inputs which contains both input data and params. You just need to differentiate param names from input data names. This can be achieved by analyzing whether a data node's `NodeEntry` is an operator's input data or param. Regarding the dependence on protobuf, onnx, and onnx-tensorrt, here is my two cents, I personally don't think introducing so many third-party dependencies is a good idea, because it results in an extra layer of complexity in terms of implementation, maintenance, build, and deployment, and the conversion process is not manageable by this community. If there is a direct solution of creating a bridge from nnvm to tensorrt, why not use it?
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on 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 With regards, Apache Git Services
