Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TF-TRT SpaceToDepth and DepthToSpace op converters for dynamic shape mode. #47590

Merged

Conversation

tfeher
Copy link
Contributor

@tfeher tfeher commented Mar 5, 2021

This PR updates the converter for the SpaceToDepth and DepthToSpace ops to work with explicit batch and dynamic shape mode.

While the explicit batch converter needs only a slight modification to insert the batch dim, the dynamic shape converter has to redefine the same target shape calculation using TRT shape tensor arithmetic.

The unit test are updated to run all three trt modes. The parameters are not modified, just refactored to allow execution in all trt modes.

Tagging @bixia1 for review and @DEKHTIARJonathan for visibility.

Tracker: #45481

@google-ml-butler google-ml-butler bot added the size:XL CL Change Size:Extra Large label Mar 5, 2021
@google-cla google-cla bot added the cla: yes label Mar 5, 2021
@gbaned gbaned self-assigned this Mar 5, 2021
@gbaned gbaned added the comp:gpu:tensorrt Issues specific to TensorRT label Mar 5, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Mar 5, 2021
@tfeher tfeher force-pushed the trt_depthspaceshuffle_dynamic branch from cdc9d9e to 4749454 Compare March 14, 2021 23:49
@bixia1 bixia1 self-requested a review March 18, 2021 21:06
@tfeher tfeher force-pushed the trt_depthspaceshuffle_dynamic branch from 4749454 to 026b878 Compare March 21, 2021 12:41
Copy link
Contributor Author

@tfeher tfeher left a comment

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.

@gbaned gbaned requested a review from bixia1 March 21, 2021 16:11
@tfeher tfeher force-pushed the trt_depthspaceshuffle_dynamic branch from 026b878 to 2b283d8 Compare March 22, 2021 22:42
Copy link
Contributor Author

@tfeher tfeher left a comment

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!

tensorflow/compiler/tf2tensorrt/convert/utils.cc Outdated Show resolved Hide resolved
tensorflow/compiler/tf2tensorrt/convert/utils.h Outdated Show resolved Hide resolved
tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc Outdated Show resolved Hide resolved
tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc Outdated Show resolved Hide resolved
@bixia1
Copy link
Contributor

bixia1 commented Mar 22, 2021

#47590 (comment)

Prefer using return values over output parameters: they improve readability, and often provide the same or better performance

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Mar 22, 2021
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Mar 22, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 22, 2021
@copybara-service copybara-service bot merged commit f3dd905 into tensorflow:master Mar 23, 2021
PR Queue automation moved this from Approved by Reviewer to Merged Mar 23, 2021
@tfeher tfeher deleted the trt_depthspaceshuffle_dynamic branch May 16, 2022 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:gpu:tensorrt Issues specific to TensorRT ready to pull PR ready for merge process size:XL CL Change Size:Extra Large
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

4 participants