Skip to content

Add TF-TRT converter for Shape op - #39990

Merged
tensorflow-copybara merged 1 commit into
tensorflow:masterfrom
tfeher:trt_shape_op
Aug 6, 2020
Merged

Add TF-TRT converter for Shape op#39990
tensorflow-copybara merged 1 commit into
tensorflow:masterfrom
tfeher:trt_shape_op

Conversation

@tfeher

@tfeher tfeher commented May 29, 2020

Copy link
Copy Markdown
Contributor

This PR adds TF-TRT converter for the shape op. To facilitate testing, the following additional changes were made:

  • Test helper routines modified to handle different output data types.
  • Test helper and profile creation routines updated to handle TRT engines that have no input tensors.

@google-ml-butler google-ml-butler Bot added the size:L CL Change Size: Large label May 29, 2020
@google-ml-butler
google-ml-butler Bot requested a review from joker-eph May 29, 2020 12:56
@gbaned gbaned self-assigned this May 29, 2020
@gbaned gbaned added the comp:gpu:tensorrt Issues specific to TensorRT label May 29, 2020
@gbaned

gbaned commented May 29, 2020

Copy link
Copy Markdown
Contributor

@tfeher Can you please resolve conflicts? Thanks!

@gbaned gbaned added the stat:awaiting response Status - Awaiting response from author label Jun 2, 2020
@gbaned

gbaned commented Jun 23, 2020

Copy link
Copy Markdown
Contributor

@tfeher Any update on this PR? Please. Thanks!

@tensorflowbutler tensorflowbutler added the stale This label marks the issue/pr stale - to be closed automatically if no activity label Jul 9, 2020
@tensorflowbutler

tensorflowbutler commented Jul 9, 2020

Copy link
Copy Markdown
Member

It has been 14 days with no activity and the awaiting response label was assigned. Is this PR still valid? Assigning the stalled label. Please comment to reassure me that this is still being worked on.

@tfeher

tfeher commented Jul 9, 2020

Copy link
Copy Markdown
Contributor Author

Sorry for the delay. I will update the PR soon. This, and many of the TF-TRT PRs that I have opened, are part of a large series of PRs that update / improve all the TF-TRT op converter tests. I will work on this as soon as some open issues are fixed in other related PRs.

@google-ml-butler google-ml-butler Bot removed the stale This label marks the issue/pr stale - to be closed automatically if no activity label Jul 9, 2020
@tensorflowbutler tensorflowbutler removed the stat:awaiting response Status - Awaiting response from author label Jul 11, 2020
@tfeher
tfeher force-pushed the trt_shape_op branch 2 times, most recently from a22b047 to e607bae Compare July 11, 2020 22:21
@tfeher tfeher mentioned this pull request Jul 12, 2020
Merged
@gbaned
gbaned requested a review from bixia1 July 14, 2020 11:52
@gbaned gbaned added the awaiting review Pull request awaiting review label Jul 14, 2020
bixia1
bixia1 reviewed Jul 20, 2020
View reviewed changes
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/utils/trt_shape_optimization_profiles.cc Resolved
@gbaned gbaned removed the awaiting review Pull request awaiting review label Jul 21, 2020
tfeher
tfeher commented Jul 21, 2020
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 addressed the issues. There is one question remaining open, whether to make the tf_type -> tf_type_ change in this PR or in a separate one.

Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/utils/trt_shape_optimization_profiles.cc Resolved
@gbaned
gbaned requested a review from bixia1 July 29, 2020 16:55
@gbaned gbaned added the awaiting review Pull request awaiting review label Aug 3, 2020
@bixia1

bixia1 commented Aug 4, 2020

Copy link
Copy Markdown
Contributor

I don't see a way to response to this, so copy the link and response here #39990 (comment)
I am fine with either renaming here or in another PR.

tfeher
tfeher commented Aug 4, 2020
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 second look. I will create a separate PR for the tf_type -> tf_type_ renaming.

Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc Resolved
@tfeher tfeher mentioned this pull request Aug 4, 2020
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 Aug 4, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 4, 2020
@google-ml-butler google-ml-butler Bot added the kokoro:force-run Tests on submitted change label Aug 4, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 4, 2020
bixia1
bixia1 requested changes Aug 4, 2020
View reviewed changes
Comment thread tensorflow/compiler/tf2tensorrt/utils/trt_shape_optimization_profiles.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc Resolved
@gbaned gbaned removed awaiting review Pull request awaiting review ready to pull PR ready for merge process labels Aug 5, 2020
@tfeher
Add TF-TRT converter for Shape op.
1033515 
The unit test includes test cases when the TensorRT network is called with no input tensor. The helper routines are adjusted to handle this case.
tfeher
tfeher commented Aug 5, 2020
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 comments, I have made addressed the issues.

Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc Resolved
Comment thread tensorflow/compiler/tf2tensorrt/utils/trt_shape_optimization_profiles.cc Resolved
@google-ml-butler google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Aug 5, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 5, 2020
@tensorflow-copybara
tensorflow-copybara merged commit 3f24d13 into tensorflow:master Aug 6, 2020
@tfeher tfeher mentioned this pull request Dec 8, 2020
Closed
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

Assignees

@gbaned gbaned

Labels

cla: yes comp:gpu:tensorrt Issues specific to TensorRT ready to pull PR ready for merge process size:L CL Change Size: Large

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@tfeher @gbaned @tensorflowbutler @bixia1 @googlebot @kokoro-team @tensorflow-copybara