-
Notifications
You must be signed in to change notification settings - Fork 6.8k
enable TensorRT integration with cpp api #15335
Conversation
ping! anyone could do a quick review? |
static const std::string TENSORRT_SUBGRAPH_PARAM_IDENTIFIER = "subgraph_params_names"; | ||
// needs to be same with | ||
// https://github.com/apache/incubator-mxnet/blob/master/src/operator/subgraph/tensorrt/tensorrt.cc#L244 | ||
static const std::string TENSORRT_SUBGRAPH_PARAM_PREFIX = "subgraph_param_"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably add those constants in src/operator/subgraph/tensorrt/tensorrt-inl.h to keep track
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean duplicate them in the tensorrt-inl.h as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was more thinking putting them in tensorrt-inl.h and including the file in this file (in case we need it somewhere).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmmm, tensorrt-inl.h is not in the default header folder. if we do that we probably need an new header file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, ignore my original comment then
b92fb25
to
2b8b640
Compare
|
||
} // namespace details | ||
|
||
namespace contrib { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@szha Hey Sheng, have we had contrib namespaces in C++ before? The intent of this code basically aligns with the intent of having a contrib level python API. Does namespacing it out like this make sense to you?
Description
This PR enables using TensorRT through cpp api
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments