Test Squeeze op converter in dynamic shape mode by tfeher · Pull Request #38146 · tensorflow/tensorflow

tfeher · 2020-04-01T22:23:18Z

This PR adds tests for dynamic shape mode for TF-TRT ConvertSqueeze.

The BuildAndRun method is extended with a profile generation step.
ConvertSqueeze is refactored: all test use TestParams
All previous test in ConvertSqueeze run both in implicit and explicit batch mode
Added new dynamic shapes tests (those run only in explicit batch mode)

PR #38118 is required for of this PR, please review that first. Tagging @bixia1 for review.

bixia1 · 2020-04-23T17:23:34Z

-  void AddTestTensor(
-      const string& name, const std::vector<int32>& dims, int batch_size = 1,
+  bool HasStaticShape(std::vector<int> dims) const {
+    return !absl::c_any_of(dims, [](int i) { return i == -1; });


The existing code uses comparison i < 0, and there is a reason for this -- if the dimension values come from graph properties, the negative values actually represent the group of the symbolic dimension value.

tensorflow/tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc

Lines 1345 to 1351 in 958d2c1

bool HasStaticShape(const nvinfer1::Dims& dims) const {

if (dims.nbDims < 0) return false;

for (int i = 0; i < dims.nbDims; ++i) {

if (dims.d[i] < 0) return false;

}

return true;

}

The dimension values come from the test parameters that we define. I do not mind changing it to i < 0 to be on the safe side. Originally, I have assumed that the dimensions represent a PartialTensorShape. Is there any valid negative value apart from -1 for a PartialTensorShape?

gbaned · 2020-04-28T11:05:24Z

@tfeher Can you please check @bixia1 comments and keep us posted. Thanks!

tfeher · 2020-04-28T18:22:48Z

Thanks @bixia1 for the review. I am working on fixing the issues, I will update soon.

…ctions

tfeher

I have addressed the the issues. As part of making the test Value-Parameterized, I have done some additional changes:

Introduced TrtTestMode enum to enumerate test options: implicit/explicit batch & dynamic shape mode.
Added more test params (apart from TrtTestMode): Converter precision and input tensor dtype.

Using these the test coverage of ConvertSqueeze is improved:

Now all the test run in dynamic shape mode (earlier only those run in dynamic shape mode where we defined partial_input_dims manually).
The converter can be tested for multiple input types and conversion precision.

Additional changes:

Previously, precision mode was set through the precision_mode_to_test_ member variable of OpConverterTest. Removed the member variable and added precision_mode_to_test as an argument of OpConverterTest::Reset(). Reset calls the constructor of the Converter and sets the TensorRT precision mode there.
Noticed that precision_mode arg of OpConverterTest::BuildAndRun is not used. Removed it.

To facilitate reusing the same set of changes for other OpConverterTests, I have moved all the inptut tensor initializing result checking code into helper routines:

AddTestTensor (with trt_mode arg),
RunValidationAndConversion,
BuildAndRunConvertedNetwork

tfeher · 2020-04-24T21:36:16Z

-  void AddTestTensor(
-      const string& name, const std::vector<int32>& dims, int batch_size = 1,
+  bool HasStaticShape(std::vector<int> dims) const {
+    return !absl::c_any_of(dims, [](int i) { return i == -1; });


The dimension values come from the test parameters that we define. I do not mind changing it to i < 0 to be on the safe side. Originally, I have assumed that the dimensions represent a PartialTensorShape. Is there any valid negative value apart from -1 for a PartialTensorShape?

bixia1

Temporarily approving this for myself to further look at this within google.

Enable dynamic shape tests for ConvertSqueeze

a4265e0

google-ml-butler Bot added the size:XL CL Change Size:Extra Large label Apr 1, 2020

google-ml-butler Bot requested a review from joker-eph April 1, 2020 22:23

googlebot added the cla: yes label Apr 1, 2020

tfeher changed the title ~~Trt squeeze op tes~~ Test Squeeze op converter in dynamic shape mode Apr 1, 2020

gbaned self-assigned this Apr 2, 2020

gbaned added comp:gpu:tensorrt Issues specific to TensorRT awaiting review Pull request awaiting review labels Apr 2, 2020

Merge branch 'master' into trt_squeeze_op_test

21666c4

gbaned requested a review from bixia1 April 17, 2020 16:13

bixia1 reviewed Apr 23, 2020

View reviewed changes

tensorflowbutler removed the awaiting review Pull request awaiting review label Apr 26, 2020

tfeher added 2 commits April 26, 2020 23:13

Improve docstring, and address issues from reviewer

7a3256d

Make ConvertSqueeze a value parameterized test

19a479d

gbaned added the stat:awaiting response Status - Awaiting response from author label Apr 28, 2020

Add more test value parameters for ConvertSqueeze, improve helper fun…

c4009c5

…ctions

tfeher commented Apr 29, 2020

View reviewed changes

gbaned removed the stat:awaiting response Status - Awaiting response from author label Apr 29, 2020

bixia1 approved these changes Apr 30, 2020

View reviewed changes

google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Apr 30, 2020

kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 30, 2020

tensorflow-copybara merged commit 609b1e8 into tensorflow:master Apr 30, 2020

tfeher mentioned this pull request Dec 8, 2020

TF-TRT Dynamic Shapes Feature Tracker #45481

Closed

tfeher deleted the trt_squeeze_op_test branch May 16, 2022 08:45

	bool HasStaticShape(const nvinfer1::Dims& dims) const {
	if (dims.nbDims < 0) return false;
	for (int i = 0; i < dims.nbDims; ++i) {
	if (dims.d[i] < 0) return false;
	}
	return true;
	}

Conversation

tfeher commented Apr 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bixia1 Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

tfeher Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gbaned commented Apr 28, 2020

Uh oh!

tfeher commented Apr 28, 2020

Uh oh!

tfeher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tfeher Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bixia1 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tfeher commented Apr 1, 2020 •

edited

Loading