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] Enable dynamic batch dim for Conv2dBackpropInput #51468

Conversation

christopherbate
Copy link
Contributor

@christopherbate christopherbate commented Aug 12, 2021

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
Copy link
Contributor Author

@bixia1
@tfeher

@tfeher
Copy link
Contributor

tfeher commented Aug 12, 2021

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
@rthadur rthadur added this to Assigned Reviewer in PR Queue via automation 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
Copy link
Contributor

bixia1 commented Aug 17, 2021

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
@christopherbate
Copy link
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
@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
Copy link
Contributor Author

christopherbate commented Aug 24, 2021

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?

@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
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer 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
PR Queue automation moved this from Approved by Reviewer to Merged 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
Labels
cla: yes comp:gpu:tensorrt Issues specific to TensorRT size:M CL Change Size: Medium
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

7 participants