Repository navigation
Improve TensorRT binding index query - #36434
Merged
tensorflow-copybara merged 8 commits intoFeb 6, 2020
Merged
Conversation
This was referenced Feb 3, 2020
bixia1
reviewed
Feb 5, 2020
| } | ||
| } | ||
|
|
||
| // Get the binding index of a tensor in an engine. |
Contributor
There was a problem hiding this comment.
Gets. Also please add a blank line after this line.
| Status GetTrtBindingIndex(const char* tensor_name, int profile_idx, | ||
| const nvinfer1::ICudaEngine* cuda_engine, | ||
| int* binding_idx) { | ||
| *binding_idx = cuda_engine->getBindingIndex(tensor_name); |
Contributor
There was a problem hiding this comment.
Can we add this comment?
// This is binding_index_within_profile.
Contributor
Author
There was a problem hiding this comment.
Described the binding index numbering scheme and explained what getBindingIndex returns.
Comment on lines
+564
to
+565
| // If we have more then one optimization profiles then the binding idx | ||
| // depends on the profile number |
Contributor
There was a problem hiding this comment.
Can we replace this comment with this:
binding_index_within_engine = binding_index_within_profile + profile_index * bindings_per_profile
| StrCat("Input node ", input_name, " not found, at ", name()); | ||
| LOG(ERROR) << msg; | ||
| ctx->SetStatus(errors::NotFound(msg)); | ||
| int binding_index = -1; |
Contributor
There was a problem hiding this comment.
Do we have to initialize binding_index here?
Contributor
Author
There was a problem hiding this comment.
No, it is not absolutely necessary. I have removed the initialization.
| // Get the binding index of a tensor in an engine. | ||
| // The binding index is looked up using the tensor's name and the profile idx. | ||
| // Profile idx should be set to zero, if we do not have optimization profiles. | ||
| Status GetTrtBindingIndex(const char* tensor_name, int profile_idx, |
Contributor
There was a problem hiding this comment.
Let's replace 'idx' with index so that we use index consistently.
tfeher
commented
Feb 6, 2020
| } | ||
| } | ||
|
|
||
| // Get the binding index of a tensor in an engine. |
| // Get the binding index of a tensor in an engine. | ||
| // The binding index is looked up using the tensor's name and the profile idx. | ||
| // Profile idx should be set to zero, if we do not have optimization profiles. | ||
| Status GetTrtBindingIndex(const char* tensor_name, int profile_idx, |
| Status GetTrtBindingIndex(const char* tensor_name, int profile_idx, | ||
| const nvinfer1::ICudaEngine* cuda_engine, | ||
| int* binding_idx) { | ||
| *binding_idx = cuda_engine->getBindingIndex(tensor_name); |
Contributor
Author
There was a problem hiding this comment.
Described the binding index numbering scheme and explained what getBindingIndex returns.
Comment on lines
+564
to
+565
| // If we have more then one optimization profiles then the binding idx | ||
| // depends on the profile number |
| StrCat("Input node ", input_name, " not found, at ", name()); | ||
| LOG(ERROR) << msg; | ||
| ctx->SetStatus(errors::NotFound(msg)); | ||
| int binding_index = -1; |
Contributor
Author
There was a problem hiding this comment.
No, it is not absolutely necessary. I have removed the initialization.
bixia1
approved these changes
Feb 6, 2020
tensorflow-bot
Bot
added
kokoro:force-run
tensorflow-copybara
pushed a commit
that referenced
this pull request
Feb 6, 2020
PiperOrigin-RevId: 293660474 Change-Id: I7868bb49ddd32ade542c8562cf89841fcdf8bf98
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR improves the TensorRT binding index query in a way that makes it compatible with optimization profiles.
Currently profiles are not used in TF-TRT, therefore this PR does not change any existing behavior. It is just a preparatory step to implement dynamic shapes with profiles.
TensorRT bindings are an array of pointers to input and output buffers for the network.
To execute TensorRT inference we have to specify the bindings.
To specify the pointers for the bindings, we have to query the binding index for each input and output tensor, and fill the data pointer in the bindings array at the correct location specified by the binding index.
Since TensorRT 6, an engine can have a optimization profiles, and each profile has it's own set of bindings. This PR make binding index query compatible with multiple optimization profiles.