[GPU] Implement ExperimentalDetectronDetectionOutput operation #11772

opoluektov-lohika · 2022-05-31T15:23:25Z

Tickets:

58493

opoluektov-lohika · 2022-05-31T15:25:11Z

Please be aware that this PR is based on the fixes in the C++ reference implementation of the operation!

This PR should be merged first: #10621

src/plugins/intel_gpu/src/graph/impls/ocl/experimental_detectron_detection_output.cpp

src/plugins/intel_gpu/src/kernel_selector/common/common_types.h

...nel_selector/core/actual_kernels/eddo/experimental_detectron_detection_output_kernel_ref.cpp

...ernel_selector/core/actual_kernels/eddo/experimental_detectron_detection_output_kernel_ref.h

...intel_gpu/src/kernel_selector/core/cl_kernels/experimental_detectron_detection_output_ref.cl

wilson-seok · 2022-06-13T00:05:19Z

@opoluektov-lohika Please resolve the confliction.

opoluektov-lohika · 2022-06-14T15:04:02Z

@wilson-seok Rebased on the latest master

...in/gpu/shared_tests_instances/single_layer_tests/experimental_detectron_detection_output.cpp

kelvinchoi-intel · 2022-06-17T05:20:46Z

src/core/reference/src/runtime/reference/experimental_detectron_detection_output.cpp

@@ -203,7 +203,7 @@ void nms_cf(const float* conf_data,

 template <typename T>
 bool SortScorePairDescend(const std::pair<float, T>& pair1, const std::pair<float, T>& pair2) {
-    return pair1.first > pair2.first;
+    return pair1.first > pair2.first || (pair1.first == pair2.first && pair1.second.second < pair2.second.second);


Use (), Ex, (pair1.first > pair2.first)

kelvinchoi-intel · 2022-06-17T05:21:02Z

src/plugins/intel_cpu/src/nodes/experimental_detectron_detection_output.cpp

-static bool SortScorePairDescend(const std::pair<float, T>& pair1, const std::pair<float, T>& pair2) {
-    return pair1.first > pair2.first;
+static bool SortScorePairDescend(const std::pair<float, std::pair<int, int>>& pair1, const std::pair<float, std::pair<int, int>>& pair2) {
+    return pair1.first > pair2.first || (pair1.first == pair2.first && pair1.second.second < pair2.second.second);


Use (), Ex, (pair1.first > pair2.first)

kelvinchoi-intel · 2022-06-17T05:22:03Z

src/plugins/intel_gpu/include/intel_gpu/primitives/experimental_detectron_detection_output.hpp

+    size_t max_detections_per_image;
+    bool class_agnostic_box_regression;
+    float max_delta_log_wh;
+    std::vector<float> deltas_weights;


Add enough comments for @param and @brief

kelvinchoi-intel · 2022-06-17T05:30:56Z

src/plugins/intel_gpu/include/intel_gpu/primitives/experimental_detectron_detection_output.hpp

+                                            float nms_threshold,
+                                            int64_t num_classes,
+                                            int64_t post_nms_count,
+                                            size_t max_detections_per_image,


Any special reason to use big sized int like int64_t and size_t? I think int32_t is enough.

Mainly I was just using the same types as in the operation.

Changed to use int

kelvinchoi-intel · 2022-06-17T05:42:24Z

src/plugins/intel_gpu/src/plugin/ops/experimental_detectron_detection_output.cpp

+                                                              inputs[2],
+                                                              inputs[3],
+                                                              inputs[4],  // output classes
+                                                              inputs[5],  // output scores


It's better to check inputs size is 6 at some point.

kelvinchoi-intel · 2022-06-17T05:45:36Z

...in/gpu/shared_tests_instances/single_layer_tests/experimental_detectron_detection_output.cpp

+
+const std::vector<ov::test::ElementType> netPrecisions = {
+    ov::element::Type_t::f16,
+    ov::element::Type_t::f32,


Please add type i32 and i64 because the kernel supports them.

If I do that, the test tries to instantiate the operation with int32/int64 boxes/deltas/scores/image_info, which is not supported, so the test fails with the following message:

.../src/tests/functional/shared_test_classes/src/base/ov_subgraph.cpp:94: Failure .../src/plugins/intel_gpu/src/plugin/program.cpp:354 cldnn program build failed! Node which is about to be added in between two other nodes should not have any existing dependencies

Shall I investigate?

I think input precision of the op is any floating type as document. So f32/f16 seems to be ok for the single layer test.

wilson-seok · 2022-06-22T06:24:40Z

...nel_selector/core/actual_kernels/eddo/experimental_detectron_detection_output_kernel_ref.cpp

+
+constexpr int kOutputIdx = 0;
+
+constexpr int kRoiCountScalarIdx = 0;


Build error happened in build_linux_android_x64. Please fix it.

The build error in build_linux_android_x64 is fixed.

wilson-seok

LGTM

wilson-seok · 2022-06-27T06:16:06Z

@opoluektov-lohika Please rebase to the latest master to solve issue of whells_windows_vs2019_release.

… stage This is to ensure the operation produces stable predictable results across the possible sorting algorithm implementaions. This property is useful for the operation testing.

…ations in kernel

…the unit test

…e outputs

…t coverage More attribute permutations were added.

opoluektov-lohika requested a review from a team as a code owner May 31, 2022 15:23

opoluektov-lohika requested a review from a team May 31, 2022 15:23

opoluektov-lohika requested review from a team as code owners May 31, 2022 15:23

opoluektov-lohika requested review from tgubanova-lohika and okhovan May 31, 2022 15:25

opoluektov-lohika force-pushed the igpu/feature/eddo branch from 7458b4d to 4690f04 Compare May 31, 2022 15:46

tgubanova-lohika reviewed Jun 1, 2022

View reviewed changes

yeonbok added category: GPU OpenVINO GPU plugin ExternalPR External contributor labels Jun 1, 2022

yeonbok added this to the 2022.2 milestone Jun 1, 2022

opoluektov-lohika force-pushed the igpu/feature/eddo branch from 4690f04 to 2b8d373 Compare June 1, 2022 15:50

tgubanova-lohika requested a review from wilson-seok June 1, 2022 19:09

wilson-seok requested review from geunhwan, jade-cho, yeonbok and andrew-k-park June 2, 2022 01:43

wilson-seok self-assigned this Jun 2, 2022

opoluektov-lohika force-pushed the igpu/feature/eddo branch 2 times, most recently from 9115833 to e342b29 Compare June 8, 2022 21:25

opoluektov-lohika force-pushed the igpu/feature/eddo branch from e342b29 to a5a3bd1 Compare June 14, 2022 12:53

wilson-seok reviewed Jun 15, 2022

View reviewed changes

...in/gpu/shared_tests_instances/single_layer_tests/experimental_detectron_detection_output.cpp Outdated Show resolved Hide resolved

...in/gpu/shared_tests_instances/single_layer_tests/experimental_detectron_detection_output.cpp Outdated Show resolved Hide resolved

opoluektov-lohika force-pushed the igpu/feature/eddo branch from a5a3bd1 to dd1a89f Compare June 15, 2022 22:24

kelvinchoi-intel reviewed Jun 17, 2022

View reviewed changes

opoluektov-lohika force-pushed the igpu/feature/eddo branch from dd1a89f to 64e11ce Compare June 20, 2022 03:06

opoluektov-lohika force-pushed the igpu/feature/eddo branch 2 times, most recently from 482a4a2 to 738de0d Compare June 20, 2022 03:23

opoluektov-lohika requested review from kelvinchoi-intel and removed request for a team June 21, 2022 13:59

wilson-seok reviewed Jun 22, 2022

View reviewed changes

opoluektov-lohika force-pushed the igpu/feature/eddo branch 2 times, most recently from fe69477 to 216fcb1 Compare June 22, 2022 18:48

wilson-seok approved these changes Jun 24, 2022

View reviewed changes

Oleksandr Poluektov added 8 commits June 27, 2022 11:46

ExperimentalDetectronDetectionOutput: refine sorting criteria for NMS…

1d58d4a

… stage This is to ensure the operation produces stable predictable results across the possible sorting algorithm implementaions. This property is useful for the operation testing.

[GPU] Implement ExperimentalDetectronDetectionOutput operation

5dca0c7

[GPU] ExperimentalDetectronDetectionOutput: use vector types and oper…

cd886a3

…ations in kernel

Reformat changed files to make clang format checker happy

5eb5e64

[GPU] ExperimentalDetectronDetectionOutput: add another test case to …

d6d0289

…the unit test

[GPU] ExperimentalDetectronDetectionOutput: Add f16 test

901931c

ExperimentalDetectronDetectionOutput: single-layer test: use all thre…

3baa260

…e outputs

[GPU] ExperimentalDetectronDetectionOutput: increase single layer tes…

bb49151

…t coverage More attribute permutations were added.

opoluektov-lohika force-pushed the igpu/feature/eddo branch from 216fcb1 to bb49151 Compare June 27, 2022 08:46

yeonbok approved these changes Jun 27, 2022

View reviewed changes

yeonbok merged commit 8a21e4e into openvinotoolkit:master Jun 27, 2022


		constexpr int kOutputIdx = 0;

		constexpr int kRoiCountScalarIdx = 0;

[GPU] Implement ExperimentalDetectronDetectionOutput operation #11772

[GPU] Implement ExperimentalDetectronDetectionOutput operation #11772

Uh oh!

Conversation

opoluektov-lohika commented May 31, 2022 • edited by wilson-seok Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tickets:

Uh oh!

opoluektov-lohika commented May 31, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wilson-seok commented Jun 13, 2022

Uh oh!

opoluektov-lohika commented Jun 14, 2022

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilson-seok left a comment

Choose a reason for hiding this comment

Uh oh!

wilson-seok commented Jun 27, 2022

Uh oh!

Uh oh!

opoluektov-lohika commented May 31, 2022 •

edited by wilson-seok

Loading