Skip to content

[TF:TRT] Always set binding dimension - #52181

Merged
copybara-service[bot] merged 1 commit into
tensorflow:masterfrom
tfeher:trt_fix_shape_io
Oct 4, 2021
Merged

[TF:TRT] Always set binding dimension#52181
copybara-service[bot] merged 1 commit into
tensorflow:masterfrom
tfeher:trt_fix_shape_io

Conversation

@tfeher

@tfeher tfeher commented Sep 29, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR fixes a bug related to shape value tensor handling and input shape specification: we need to set binding dimensions even if tensor is not an execution tensor

Background. According to nvinfer1::ITensor::isExecutionTensor API description:

A tensor with isShapeTensor() == false and isExecutionTensor() == false can still show up as an input to the engine if its dimensions are required.

This PR fixes optimization profile definition for that case, by always calling execution_context->setBindingDimension().

A unit test is added to for such a case.

Tracker #45481

Tagging @bixia1 for review.

@google-ml-butler google-ml-butler Bot added the size:M CL Change Size: Medium label Sep 29, 2021
@google-ml-butler google-ml-butler Bot added the awaiting review Pull request awaiting review label Sep 29, 2021
@google-cla google-cla Bot added the cla: yes label Sep 29, 2021
@bixia1
bixia1 self-requested a review September 29, 2021 15:03
@tfeher tfeher mentioned this pull request Sep 29, 2021
Closed
bixia1
bixia1 reviewed Sep 29, 2021
View reviewed changes
Comment thread tensorflow/python/compiler/tensorrt/test/shape_output_test.py Resolved
Comment thread tensorflow/python/compiler/tensorrt/test/shape_output_test.py Resolved
Comment thread tensorflow/python/compiler/tensorrt/test/shape_output_test.py Resolved
Comment thread tensorflow/compiler/tf2tensorrt/utils/trt_shape_optimization_profiles.cc Resolved
@gbaned gbaned self-assigned this Sep 30, 2021
@gbaned gbaned added the comp:gpu:tensorrt Issues specific to TensorRT label Sep 30, 2021
tfeher
tfeher commented Sep 30, 2021
View reviewed changes

@tfeher tfeher left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bixia1 for the review. I have answered the questions. I still need to fix the issue with n_inputs. I will send an update soon.

Comment thread tensorflow/python/compiler/tensorrt/test/shape_output_test.py Resolved
Comment thread tensorflow/python/compiler/tensorrt/test/shape_output_test.py Resolved
Comment thread tensorflow/python/compiler/tensorrt/test/shape_output_test.py Resolved
Comment thread tensorflow/compiler/tf2tensorrt/utils/trt_shape_optimization_profiles.cc Resolved
@tfeher
tfeher force-pushed the trt_fix_shape_io branch 2 times, most recently from cabffc3 to 629b402 Compare September 30, 2021 17:58
tfeher
tfeher commented Sep 30, 2021
View reviewed changes

@tfeher tfeher left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed the incomplete fix for pruned input. @bixia1 ready for review.

@tfeher tfeher changed the title [TF:TRT] Fix shape tensor handling [TF:TRT] Always set binding dimension Sep 30, 2021
@tfeher tfeher mentioned this pull request Sep 30, 2021
Merged
@google-ml-butler google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Sep 30, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Sep 30, 2021
@tfeher tfeher mentioned this pull request Sep 30, 2021
Merged
@sanjoy
sanjoy removed their request for review October 1, 2021 03:35
@gbaned gbaned removed the awaiting review Pull request awaiting review label Oct 1, 2021
@copybara-service
copybara-service Bot merged commit 3a7ca7a into tensorflow:master Oct 4, 2021
@google-ml-butler google-ml-butler Bot removed the ready to pull PR ready for merge process label Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bixia1 bixia1 bixia1 approved these changes
@joker-eph joker-eph Awaiting requested review from joker-eph
@sherhut sherhut Awaiting requested review from sherhut

Assignees

@gbaned gbaned

Labels

cla: yes comp:gpu:tensorrt Issues specific to TensorRT size:M CL Change Size: Medium

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tfeher @bixia1 @kokoro-team @gbaned