Skip to content

[TFTRT - Dynamic Shape Phase 3] Add Dynamic Shape Testing for ConvertClipByValue - #45589

Merged
copybara-service[bot] merged 1 commit into
tensorflow:masterfrom
DEKHTIARJonathan:jdekhtiar/dynamic_shape/ConvertClipByValue
Dec 16, 2020
Merged

[TFTRT - Dynamic Shape Phase 3] Add Dynamic Shape Testing for ConvertClipByValue#45589
copybara-service[bot] merged 1 commit into
tensorflow:masterfrom
DEKHTIARJonathan:jdekhtiar/dynamic_shape/ConvertClipByValue

Conversation

@DEKHTIARJonathan

@DEKHTIARJonathan DEKHTIARJonathan commented Dec 11, 2020

Copy link
Copy Markdown
Contributor

@bixia1 @tfeher for review

Feature Tracker: #45481

Please merge this PR after: #45587

@google-ml-butler google-ml-butler Bot added the size:M CL Change Size: Medium label Dec 11, 2020
@google-ml-butler
google-ml-butler Bot requested a review from joker-eph December 11, 2020 00:23
@google-cla google-cla Bot added the cla: yes label Dec 11, 2020
@gbaned gbaned self-assigned this Dec 11, 2020
@gbaned gbaned added the comp:gpu:tensorrt Issues specific to TensorRT label Dec 11, 2020
@gbaned
gbaned requested a review from bixia1 December 11, 2020 02:38
bixia1
bixia1 reviewed Dec 11, 2020
View reviewed changes

@bixia1 bixia1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. We will come back to this after the parent PR is merged.

@DEKHTIARJonathan

DEKHTIARJonathan commented Dec 12, 2020

Copy link
Copy Markdown
Contributor Author

@bixia1 rebased on master (with the new OpConverterTest name).
Good for review / merge

@google-ml-butler google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Dec 14, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 14, 2020
@bixia1

bixia1 commented Dec 14, 2020

Copy link
Copy Markdown
Contributor

The test doesn't even compiled and here is just one example of the error message:
third_party/tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc:6637:5: error: no matching member function for call to 'AddTestTensor'
AddTestTensor("t", {1, 2, 3}, trt_type_);
^~~~~~~~~~~~~
third_party/tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc:1811:8: note: candidate template ignored: could not match 'vector' against 'nvinfer1::DataType'
void AddTestTensor(const string& name, const std::vector& dims,
^

@gbaned gbaned removed the ready to pull PR ready for merge process label Dec 14, 2020
@DEKHTIARJonathan

DEKHTIARJonathan commented Dec 15, 2020

Copy link
Copy Markdown
Contributor Author

The bug is fixed and we are good.

INFO: 318 processes: 318 local.
INFO: Build completed successfully, 325 total actions
//tensorflow/compiler/tf2tensorrt:convert_graph_test                     PASSED in 16.7s
//tensorflow/compiler/tf2tensorrt:convert_graph_test_gpu                 PASSED in 14.9s
//tensorflow/compiler/tf2tensorrt:convert_nodes_test                     PASSED in 96.0s
//tensorflow/compiler/tf2tensorrt:convert_nodes_test_gpu                 PASSED in 86.8s
//tensorflow/compiler/tf2tensorrt:segment_test                           PASSED in 0.2s
//tensorflow/compiler/tf2tensorrt:segment_test_gpu                       PASSED in 0.1s
//tensorflow/compiler/tf2tensorrt:tensorrt_test_cc                       PASSED in 16.7s
//tensorflow/compiler/tf2tensorrt:tensorrt_test_cc_gpu                   PASSED in 15.9s
//tensorflow/compiler/tf2tensorrt:trt_allocator_test                     PASSED in 0.1s
//tensorflow/compiler/tf2tensorrt:trt_engine_op_test                     PASSED in 13.0s
//tensorflow/compiler/tf2tensorrt:trt_engine_op_test_gpu                 PASSED in 12.4s
//tensorflow/compiler/tf2tensorrt:trt_engine_resource_ops_test           PASSED in 13.1s
//tensorflow/compiler/tf2tensorrt:trt_engine_resource_ops_test_gpu       PASSED in 12.4s
//tensorflow/compiler/tf2tensorrt:trt_lru_cache_test                     PASSED in 0.1s
//tensorflow/compiler/tf2tensorrt:trt_shape_optimization_profiles_test   PASSED in 15.9s
//tensorflow/compiler/tf2tensorrt:trt_shape_optimization_profiles_test_gpu PASSED in 12.1s

Executed 16 out of 16 tests: 16 tests pass.
INFO: Build completed successfully, 325 total actions

@google-ml-butler google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Dec 15, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 15, 2020
@bixia1

bixia1 commented Dec 15, 2020

Copy link
Copy Markdown
Contributor

I am still seeing this error when using TRT7:
third_party/tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc:6687:44: error: type 'float' cannot be narrowed to 'int' in initializer list [-Wc++11-narrowing]
AddTestWeights("clip_value_min", {1}, {p.clip_value_min}, tf_type_);
^~~~~~~~~~~~~~~~
third_party/tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc:6687:44: note: insert an explicit cast to silence this issue
AddTestWeights("clip_value_min", {1}, {p.clip_value_min}, tf_type_);
^~~~~~~~~~~~~~~~
static_cast( )

@google-ml-butler google-ml-butler Bot removed the ready to pull PR ready for merge process label Dec 15, 2020
@DEKHTIARJonathan

DEKHTIARJonathan commented Dec 15, 2020

Copy link
Copy Markdown
Contributor Author

Hum... I'm not exactly sure why it fails on your side. It works perfectly on my end...

I changed the dtype of clip_value_min and clip_value_max to int. That should fix the error message you saw: https://github.com/tensorflow/tensorflow/pull/45589/files#diff-b9ae60f058d9ae72ddc0625fa93ff1b8441e57db5695e8444d18becdb3abea2dR6655-R6656

INFO: Build completed successfully, 21 total actions
//tensorflow/compiler/tf2tensorrt:convert_graph_test                     PASSED in 14.7s
//tensorflow/compiler/tf2tensorrt:convert_graph_test_gpu                 PASSED in 14.6s
//tensorflow/compiler/tf2tensorrt:convert_nodes_test                     PASSED in 91.2s
//tensorflow/compiler/tf2tensorrt:convert_nodes_test_gpu                 PASSED in 91.8s
//tensorflow/compiler/tf2tensorrt:segment_test                           PASSED in 0.3s
//tensorflow/compiler/tf2tensorrt:segment_test_gpu                       PASSED in 0.1s
//tensorflow/compiler/tf2tensorrt:tensorrt_test_cc                       PASSED in 13.2s
//tensorflow/compiler/tf2tensorrt:tensorrt_test_cc_gpu                   PASSED in 13.1s
//tensorflow/compiler/tf2tensorrt:trt_allocator_test                     PASSED in 0.1s
//tensorflow/compiler/tf2tensorrt:trt_engine_op_test                     PASSED in 12.6s
//tensorflow/compiler/tf2tensorrt:trt_engine_op_test_gpu                 PASSED in 12.7s
//tensorflow/compiler/tf2tensorrt:trt_engine_resource_ops_test           PASSED in 12.4s
//tensorflow/compiler/tf2tensorrt:trt_engine_resource_ops_test_gpu       PASSED in 12.2s
//tensorflow/compiler/tf2tensorrt:trt_lru_cache_test                     PASSED in 0.1s
//tensorflow/compiler/tf2tensorrt:trt_shape_optimization_profiles_test   PASSED in 12.1s
//tensorflow/compiler/tf2tensorrt:trt_shape_optimization_profiles_test_gpu PASSED in 12.0s

Executed 16 out of 16 tests: 16 tests pass.
INFO: Build completed successfully, 21 total actions

@google-ml-butler google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Dec 16, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 16, 2020
@copybara-service
copybara-service Bot merged commit 66f670d into tensorflow:master Dec 16, 2020
@DEKHTIARJonathan
DEKHTIARJonathan deleted the jdekhtiar/dynamic_shape/ConvertClipByValue branch January 14, 2021 01:28
@tfeher tfeher mentioned this pull request Apr 1, 2021
Closed
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

Assignees

@gbaned gbaned

Labels

cla: yes comp:gpu:tensorrt Issues specific to TensorRT ready to pull PR ready for merge process size:M CL Change Size: Medium

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@DEKHTIARJonathan @bixia1 @kokoro-team @gbaned