reminisce commented on a change in pull request #11325: [MXNET-703] TensorRT
runtime integration
URL: https://github.com/apache/incubator-mxnet/pull/11325#discussion_r209037851
##########
File path: src/executor/graph_executor.cc
##########
@@ -56,8 +56,8 @@ GraphExecutor::~GraphExecutor() {
}
}
-inline NDArray InitZeros(const NDArrayStorageType stype, const TShape &shape,
- const Context &ctx, const int dtype) {
+inline NDArray GraphExecutor::InitZeros(const NDArrayStorageType stype, const
TShape &shape,
Review comment:
I can see the reasoning of making these free util functions as member
functions of graph executor after reading the trt executor implementation.
However, this kind of encapsulation is unnecessary and against the goal of
cutting component design dependencies. These util functions can be reused by
any other modules/functions/classes other than graph executor. Can we put them
in a util file as free functions instead? See here for the argument of
preferring free functions to member functions.
https://stackoverflow.com/questions/5989734/effective-c-item-23-prefer-non-member-non-friend-functions-to-member-functions
----------------------------------------------------------------
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
