TF-TRT Improve reshape op converter by tfeher · Pull Request #40545 · tensorflow/tensorflow

tfeher · 2020-06-17T12:59:11Z

This PR improves the Reshape op converter to handle explicit batch and dynamic shape input data.

Since TRT6, IShuffleLayer has an option to accept the shape parameter as an input tensor. If the shape input tensor corresponds to an input tensor of the TRTEngineOp, then we need to specify the input shape values for the engine. Additionally we need to specify optimization profile parameters for the shape values. The necessary changes were implemented in TrtShapeOptimizationProfile, TRTEngineOp, trt_engine_utils.

The unit tests have only a single op, and therefore handling shape input values are required to test
reshape with tensor input shape. In normal network it is less likely to have shape input tensors.

Additionally the handling of scalar input/output shapes are improved.

C++ test for serialization / deserialization of input shapes and shape values added.

Note: TensorRT has a function INetworkDefinition::markOutputForShapes which could enable the output tensor's value to be computed by IExecutionContext::getShapeBinding. This is not used in this PR, but it might be useful for shape inference.

gbaned · 2020-07-14T15:11:04Z

@tfeher This PR is in draft, any update on this? Please. Thanks!

tfeher · 2020-07-20T15:07:45Z

I am working on an update that would enable the missing python test for this feature. It shall be ready by next week.

gbaned · 2020-08-13T16:02:59Z

@tfeher This PR is in draft, any update on this? Please. Thanks!

tfeher · 2020-08-26T11:42:53Z

I have updated the PR, it is ready for review.

gbaned · 2020-09-15T16:16:39Z

@tfeher Can you please check @bixia1's comments and keep us posted ? Thanks!

gbaned · 2020-09-23T09:38:41Z

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

tfeher · 2020-09-23T15:50:46Z

There is an issue with the converter using TRT 7.1.3. I am working on a fix.

gbaned · 2020-10-09T13:30:50Z

@tfeher Any update on this PR? and please resolve conflicts Thanks!

tfeher · 2020-10-15T16:45:33Z

The unit test of the converter fails in explicit batch mode with TRT 7.1.3 due to a bug in TRT. Waiting for feedback from the TRT team.

bixia1 · 2021-03-08T03:53:45Z

I got three tests failed:
(1) tensorflow/python/compiler/tensorrt:gpu_combined_nms_test. message is like
File "/build/work/fe0e3a7166e31378da05c1404a4fcd4fedb1/google3/runfiles/google3/third_party/tensorflow/python/compiler/tensorrt/test/tf_trt_integration_test_base.py", line 1107, in _Test
self.RunTest(run_params)
File "/build/work/fe0e3a7166e31378da05c1404a4fcd4fedb1/google3/runfiles/google3/third_party/tensorflow/python/compiler/tensorrt/test/tf_trt_integration_test_base.py", line 1030, in RunTest
GraphState.INFERENCE)
File "/build/work/fe0e3a7166e31378da05c1404a4fcd4fedb1/google3/runfiles/google3/third_party/tensorflow/python/compiler/tensorrt/test/tf_trt_integration_test_base.py", line 907, in _VerifyGraphDef
graph_state)
File "/build/work/fe0e3a7166e31378da05c1404a4fcd4fedb1/google3/runfiles/google3/third_party/tensorflow/python/compiler/tensorrt/test/tf_trt_integration_test_base.py", line 893, in _VerifyGraphDefV2
[name for name in unexpected_names if name in all_op_names])
AssertionError: ['score_threshold', 'max_output_size_per_class', 'iou_threshold', 'max_total_size'] has length of 4.
(2) tensorflow/python/compiler/tensorrt:gpu_neighboring_engine_test
and tensorflow/python/compiler/tensorrt:gpu_multi_connection_neighbor_engine_test
they both failed because of native segment execution happened,
(0) Aborted: User disallowed engine native segment execution
[[{{node TRTEngineOp_0_1}}]]

bixia1 · 2021-03-08T03:57:11Z

tensorflow/compiler/tf2tensorrt:convert_nodes_test_gpu also failed
test.log

tfeher

Thanks @bixia1 for the review. I have corrected the smaller issues. I am looking into the python test problems.

The C++ test seem to have a problem with ConvertConv3D. I could not reproduce that wint TRT 7.2.1, but I will have a look with other versions.

There seem to be an additional issue with shape value handling in TRT6. I will investigate, and disable conversion in the affected versions.

tfeher · 2021-03-08T23:49:18Z

I have disabled ConvertDynamicReshape before TRT 7.1.3, this way the reshape unit test works in all TRT versions that I have tested (5.1.5, 6.0.1, 7.1.3 and 7.2.1).
I could not reproduce the Conv3D test with TRT 7.1.3.4.
I confirm the problems with the two neighbor engine tests. I will continue to investigate those.

bixia1

Per offline conversation, approving this for testing inside google while Tamas is still working on the python tests.

bixia1 · 2021-03-09T07:23:19Z

I found the reason for the Conv3D failure. This line of code is accessing garbage value when c_index == 4, and for implicit batch mode.
In case you need a test to verify it, here is one.
The problem is actually not introduced by this PR, but this one.

tfeher · 2021-03-09T18:12:52Z

Thanks @bixia1 for looking into the Conv3D failure, I will add a fix in this PR.

The last commit seems to solve the problem with the with the two neighbor engine tests. There is still an issue with the combined_nms_test.py, I am looking into that.

bixia1

Approving this so that I can get it to google and test it while Tamas is still working on one failing test.

- Add profile management for shape input tensors - Allow scalars for TRT6 or above - Improve engine serialization test. Additionally improve shape value handling. Adapt to Shape IO API change in TRT 7.1.3. - Disable ConvertDynamicReshape befor TRT 7.1.3 - Fix Conv3D channel index in implicit batch mode - Update CombinedNMS scalar value check

tfeher

The NMS Python test error was caused by a change in how scalars are represented in this PR. I have updated the converter, the test passes now.

I have also fixed the two smaller issues.

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

google-ml-butler Bot requested a review from joker-eph June 17, 2020 12:59

googlebot added the cla: yes label Jun 17, 2020

tfeher marked this pull request as draft June 17, 2020 12:59

gbaned self-assigned this Jun 17, 2020

gbaned added the comp:gpu:tensorrt Issues specific to TensorRT label Jun 17, 2020

tfeher force-pushed the trt_reshape_dynamic branch from 318b5c4 to 8f64eab Compare June 19, 2020 16:34

tfeher mentioned this pull request Jul 20, 2020

TF-TRT Slice op converters explicit batch mode #40736

Merged

bixia1 self-requested a review July 21, 2020 00:00

tfeher force-pushed the trt_reshape_dynamic branch 2 times, most recently from 35f68f2 to f6f0f81 Compare August 26, 2020 11:40

tfeher marked this pull request as ready for review August 26, 2020 11:42

gbaned requested review from bixia1 and removed request for bixia1 September 1, 2020 17:11

gbaned added the awaiting review Pull request awaiting review label Sep 1, 2020

bixia1 reviewed Sep 10, 2020

View reviewed changes

Comment thread tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc Resolved

gbaned added stat:awaiting response Status - Awaiting response from author and removed awaiting review Pull request awaiting review labels Sep 11, 2020

tensorflowbutler removed the stat:awaiting response Status - Awaiting response from author label Sep 25, 2020

gbaned added the stat:awaiting response Status - Awaiting response from author label Sep 25, 2020

tensorflowbutler removed the stat:awaiting response Status - Awaiting response from author label Oct 17, 2020

google-ml-butler Bot added the ready to pull PR ready for merge process label Mar 8, 2021

kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 8, 2021

bixia1 requested changes Mar 8, 2021

View reviewed changes

google-ml-butler Bot removed the ready to pull PR ready for merge process label Mar 8, 2021

tfeher commented Mar 8, 2021

View reviewed changes

bixia1 approved these changes Mar 9, 2021

View reviewed changes

google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Mar 9, 2021

kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 9, 2021

google-ml-butler Bot removed the ready to pull PR ready for merge process label Mar 9, 2021

bixia1 approved these changes Mar 9, 2021

View reviewed changes

Comment thread tensorflow/compiler/tf2tensorrt/BUILD Resolved

Comment thread tensorflow/compiler/tf2tensorrt/utils/trt_shape_optimization_profiles.cc Resolved

google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Mar 9, 2021

kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 9, 2021

tfeher force-pushed the trt_reshape_dynamic branch from 23dd7c8 to 47ef8f0 Compare March 9, 2021 22:29

google-ml-butler Bot removed the ready to pull PR ready for merge process label Mar 9, 2021

tfeher commented Mar 9, 2021

View reviewed changes

Comment thread tensorflow/compiler/tf2tensorrt/BUILD Resolved

Comment thread tensorflow/compiler/tf2tensorrt/utils/trt_shape_optimization_profiles.cc Resolved

bixia1 approved these changes Mar 9, 2021

View reviewed changes

google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Mar 9, 2021

kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 9, 2021

copybara-service Bot merged commit ce9f460 into tensorflow:master Mar 10, 2021

tfeher mentioned this pull request Mar 22, 2021

TF-TRT Fix batch dim for unit test with scalar input #47982

Merged

tfeher deleted the trt_reshape_dynamic branch May 16, 2022 08:46

Conversation

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

Uh oh!

Uh oh!

gbaned commented Jul 14, 2020

Uh oh!

tfeher commented Jul 20, 2020

Uh oh!

gbaned commented Aug 13, 2020

Uh oh!

tfeher commented Aug 26, 2020

Uh oh!

Uh oh!

gbaned commented Sep 15, 2020

Uh oh!

gbaned commented Sep 23, 2020

Uh oh!

tfeher commented Sep 23, 2020

Uh oh!

gbaned commented Oct 9, 2020

Uh oh!

tfeher commented Oct 15, 2020

Uh oh!

bixia1 commented Mar 8, 2021

Uh oh!

bixia1 commented Mar 8, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tfeher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tfeher commented Mar 8, 2021

Uh oh!

bixia1 left a comment

Choose a reason for hiding this comment

Uh oh!

bixia1 commented Mar 9, 2021

Uh oh!

tfeher commented Mar 9, 2021

Uh oh!

bixia1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tfeher left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tfeher commented Jun 17, 2020 •

edited

Loading

tfeher left a comment •

edited

Loading