mkolod commented on a change in pull request #11325: [MXNET-703] TensorRT
runtime integration
URL: https://github.com/apache/incubator-mxnet/pull/11325#discussion_r205226084
##########
File path: include/mxnet/executor.h
##########
@@ -152,14 +152,14 @@ class Executor {
static Executor* SimpleBind(nnvm::Symbol symbol,
Review comment:
@piiswrong I think we already discussed this
[here](https://github.com/apache/incubator-mxnet/pull/11325/files/6ce43cb90658e8cb13177a0210093b567e33e4f6#r197672536).
The TensorRT graph pass does a graph rewrite, and that requires mutable types.
Going from const ref to ref causes the linter to fail. The linter suggestion is
to switch from const refs to pointers if mutability is required. You mentioned
[here](https://github.com/apache/incubator-mxnet/pull/11325/files/6ce43cb90658e8cb13177a0210093b567e33e4f6#r198296467)
that this was acceptable from an API stability point of view, because no
changes in the C API are necessary, only in the C++ one.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
