Skip to content

[TF:TRT] Enable dynamic batch dim for Conv2dBackpropInput - #51468

Merged
copybara-service[bot] merged 1 commit into
tensorflow:masterfrom
christopherbate:tf-trt-enable-dynamic-batch-size-on-conv2dbackpropinput
Aug 26, 2021
Merged

[TF:TRT] Enable dynamic batch dim for Conv2dBackpropInput#51468
copybara-service[bot] merged 1 commit into
tensorflow:masterfrom
christopherbate:tf-trt-enable-dynamic-batch-size-on-conv2dbackpropinput

Conversation

@christopherbate

@christopherbate christopherbate commented Aug 12, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR enables dynamic batch dimension for Conv2dBackpropInput.

We enable dynamic batch dimension for Conv2DBackpropInput simply by relaxing the check to allow dynamic batch dimension in the converter. We also add positive and negative tests.

The support for dynamic height and width will come later. This is because we will need to add an additional dynamic padding operation in order to support dynamic height and width for Conv2dBackpropinput.

@google-ml-butler google-ml-butler Bot added the size:M CL Change Size: Medium label Aug 12, 2021
@google-ml-butler google-ml-butler Bot added the awaiting review Pull request awaiting review label Aug 12, 2021
@google-cla google-cla Bot added the cla: yes label Aug 12, 2021
@christopherbate

christopherbate commented Aug 12, 2021

Copy link
Copy Markdown
Contributor Author

@bixia1
@tfeher

@christopherbate christopherbate mentioned this pull request Aug 12, 2021
Closed
@tfeher

tfeher commented Aug 12, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks @christopherbate for this PR! I had a quick look at the changes, it looks good overall.

@rthadur rthadur self-assigned this Aug 13, 2021
@gbaned gbaned added the comp:gpu:tensorrt Issues specific to TensorRT label Aug 17, 2021
@gbaned gbaned assigned gbaned and unassigned rthadur Aug 17, 2021
@gbaned
gbaned requested a review from bixia1 August 17, 2021 06:20
@bixia1

bixia1 commented Aug 17, 2021

Copy link
Copy Markdown
Contributor

This is what you have for the PR description:
This PR enables dynamic batch dimension for Conv2dBackpropInput.

Full dynamic shape for height and width will come later. We first need to add an additional dynamic padding operation support in order to handle the case where the input_shapes operation argument is non-constant.

Here is my suggestion to revise it. Do you see any problem?
This PR enables dynamic batch dimension for Conv2dBackpropInput.

We enable dynamic batch dimension for Conv2DBackpropInput simply by relaxing the check to allow dynamic batch dimension in the converter. We also add positive and negative tests.

The support for dynamic height and width will come later. This is because we will need to add an additional dynamic padding operation in order to support dynamic height and width for Conv2dBackpropinput.

@bixia1 bixia1 changed the title TF-TRT: Enable dynamic batch dim for Conv2dBackpropInput [TF:TRT] Enable dynamic batch dim for Conv2dBackpropInput Aug 17, 2021
bixia1
bixia1 reviewed Aug 17, 2021
View reviewed changes
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
@christopherbate

christopherbate commented Aug 17, 2021

Copy link
Copy Markdown
Contributor Author

This is what you have for the PR description:
This PR enables dynamic batch dimension for Conv2dBackpropInput.

Full dynamic shape for height and width will come later. We first need to add an additional dynamic padding operation support in order to handle the case where the input_shapes operation argument is non-constant.

Here is my suggestion to revise it. Do you see any problem?
This PR enables dynamic batch dimension for Conv2dBackpropInput.

We enable dynamic batch dimension for Conv2DBackpropInput simply by relaxing the check to allow dynamic batch dimension in the converter. We also add positive and negative tests.

The support for dynamic height and width will come later. This is because we will need to add an additional dynamic padding operation in order to support dynamic height and width for Conv2dBackpropinput.

No problem, I updated the description.

@sanjoy
sanjoy removed their request for review August 19, 2021 04:31
@tensorflowbutler tensorflowbutler removed the awaiting review Pull request awaiting review label Aug 21, 2021
@christopherbate
christopherbate force-pushed the tf-trt-enable-dynamic-batch-size-on-conv2dbackpropinput branch 2 times, most recently from f92cfac to 2355bd3 Compare August 24, 2021 22:47
bixia1
bixia1 reviewed Aug 24, 2021
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.cc Resolved
@christopherbate
christopherbate force-pushed the tf-trt-enable-dynamic-batch-size-on-conv2dbackpropinput branch 2 times, most recently from 318ddf7 to 526ef46 Compare August 24, 2021 23:34
@christopherbate
christopherbate force-pushed the tf-trt-enable-dynamic-batch-size-on-conv2dbackpropinput branch from 526ef46 to 638e3f3 Compare August 24, 2021 23:41
@christopherbate

christopherbate commented Aug 24, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I updated test parameter naming to be inline with other dynamic shape developments.

I've rebased on master, but it still it shows difference between third_party/tf_runtime/workspace.bzl. Any ideas on why this is?

@christopherbate christopherbate mentioned this pull request Aug 25, 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 Aug 25, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 25, 2021
@copybara-service
copybara-service Bot merged commit 760f923 into tensorflow:master Aug 26, 2021
@google-ml-butler google-ml-butler Bot removed the ready to pull PR ready for merge process label Aug 26, 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.

7 participants

@christopherbate @tfeher @bixia1 @kokoro-team @tensorflowbutler @rthadur @gbaned