G-API: ONNX. Disable unnecessary outputs by mpashchenkov · Pull Request #18903 · opencv/opencv

mpashchenkov · 2020-11-23T16:41:12Z

Disable unnecessary outputs

How it works:

Unnecessary network outputs can be omitted. For example: FRCNN has output with INT_64 data type, which unsupported for OCV. Name of this output is "6381". We can "disable" this output. It works for dynamic output case only when we should remap onnx outputs to g-api out.
You can set layer names which you will use in remapping. You should call overload of cfgPostProc function for this.

.cfgOutputLayers({"gapi_out1", "gapi_out2"})
.cfgPostProc({cv::GMatDesc{CV_32F, {1,20}}, cv::GMatDesc{CV_32F, {5}}},
             remapRcnnPorts,
             {"6383", "6379"}
);

There are two layers with names "6383" and "6379" will be used for remapping. Data will be remapped from "6383", "6379" to "gapi_out1", "gapi_out2" with remapRcnnPorts function logic (Unfortunately we can't set out_meta for network with dynamic outputs, but we can set own outputs and remap data to them from onnx result).
Disabling is independent of cfgOutputLayers function in this case.

For CNN with static outputs use another logic. We know out_meta and don't need remapping in tis case.

Tests:

Test contains infer for FRCNN. FRCNN output with name "6381" has unsupported type.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
The PR is proposed to proper branch
There is reference to original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

Custom build:

force_builders=Custom
buildworker:Custom=linux-1
build_image:Custom=ubuntu-onnx:20.04
test_modules:Custom=gapi
test_filter:Custom=*ONNX*

OrestChura

Well done!

modules/gapi/include/opencv2/gapi/infer/onnx.hpp

modules/gapi/src/backends/onnx/gonnxbackend.cpp

modules/gapi/test/infer/gapi_infer_onnx_test.cpp

modules/gapi/src/backends/onnx/gonnxbackend.cpp

modules/gapi/test/infer/gapi_infer_onnx_test.cpp

OrestChura · 2020-12-01T00:06:11Z

Would you also change the PR's description, please?

asmorkalov · 2020-12-09T14:27:48Z

@OrestChura @mpashchenkov Friendly reminder.

OrestChura

LGTM!

alalek · 2020-12-11T16:38:04Z

modules/gapi/test/infer/gapi_infer_onnx_test.cpp

+
+TEST_F(ONNXWithRemap, InferWithDisabledOut)
+{
+    useModel("/object_detection_segmentation/faster-rcnn/model/FasterRCNN-10");


remove leading /

alalek · 2020-12-11T19:00:56Z

modules/gapi/include/opencv2/gapi/infer/onnx.hpp


    std::vector<bool> normalize;
+
+    std::vector<std::string> d_out_names;


It make sense to add comment with decoding of field name

Added comment.

alalek · 2020-12-11T19:02:04Z

modules/gapi/src/backends/onnx/gonnxbackend.cpp

+            ti.is_disabled = (std::find(params.d_out_names.begin(),
+            params.d_out_names.end(), ti.name) != params.d_out_names.end()) && is_postproc;


fix indentation

dmatveev · 2020-12-14T13:58:50Z

.cfgDisableOutput( { with_layer's_name_that_should_be_disabled, ..., ... } )

Can't we achieve the same by calling cfgOutputLayers and NOT mentioning that particular output layer there?

mpashchenkov · 2020-12-15T15:11:54Z

@dmatveev, unfortunately no. We use remap functions; ssd-mobilenet, for example, returns four outputs, but we call cfgOutputLayers("output_name") with one name (sample and test). Now, if user needs only 3 out of 4 outputs in remap, he can disable the unnecessary. And it works only when we use remap functions (with dynamic outputs).

modules/gapi/include/opencv2/gapi/infer/onnx.hpp

modules/gapi/src/backends/onnx/gonnxbackend.cpp

modules/gapi/test/infer/gapi_infer_onnx_test.cpp

TolyaTalamanov · 2021-02-24T09:41:42Z

Let's discuss it locally

modules/gapi/src/backends/onnx/gonnxbackend.cpp

modules/gapi/include/opencv2/gapi/infer/onnx.hpp

modules/gapi/src/backends/onnx/gonnxbackend.cpp

modules/gapi/test/infer/gapi_infer_onnx_test.cpp

TolyaTalamanov · 2021-03-18T12:56:48Z

modules/gapi/src/backends/onnx/gonnxbackend.cpp

+            if (original_data != received_data) {
+                cv::util::throw_error
+                    (std::logic_error
+                     ("OpenCV kernel output parameter was reallocated. \n"


Does it make sense to put information that it happened in ONNX backend ?

Changed message. Added mention about ONNX.

TolyaTalamanov · 2021-03-18T14:02:35Z

modules/gapi/test/infer/gapi_infer_onnx_test.cpp

+              gapi_out.begin<T>());
+    if (gapi_out.total() > onnx_out.total()) {
+        T* gptr = gapi_out.ptr<T>();
+        gptr[size] = static_cast<T>(end_mark);


What's the reason of using static_cast ? end_mark and gptr has the same type

No sense. Fixed.

mpashchenkov · 2021-03-22T14:07:44Z

@alalek, can you merge this PR?

dmatveev · 2021-03-29T13:35:21Z

modules/gapi/include/opencv2/gapi/infer/onnx.hpp

+    Params<Net>& cfgPostProc(std::vector<cv::GMatDesc> &&out_metas,
+                             PostProc &&pp) {
+        desc.out_metas        = std::move(out_metas);
+        desc.custom_post_proc = std::move(pp);
+        return *this;
+    }


I believe doxygen comments are really missing here since the semantics of these methods is not very trivial

modules/gapi/test/infer/gapi_infer_onnx_test.cpp

dmatveev

👍 follow-up is required on the documentation update, but it shouldn't block the merge.

dmatveev · 2021-03-31T12:52:08Z

modules/gapi/include/opencv2/gapi/infer/onnx.hpp

+    /** @brief Configures graph output and sets the post processing function from user.
+
+    The function is used for the case of infer of networks with dynamic outputs.
+    Since these networks haven't known output parameters needs provide them for
+    construction of output of graph.
+    The function provides meta information of outputs and post processing function.
+    Post processing function is used for copy information from ONNX infer's result
+    to output of graph which is allocated by out meta information.
+
+    @param out_metas out meta information.
+    @param pp post processing function, which has two parameters. First is onnx
+    result, second is graph output. Both parameters is std::map that contain pair of
+    layer's name and cv::Mat.
+    @return reference to object of class Params.
+    */


Please merge this now open a follow-up task to fix this text.

mpashchenkov added test category: g-api / gapi labels Nov 23, 2020

mpashchenkov requested review from OrestChura and TolyaTalamanov November 23, 2020 16:41

OrestChura reviewed Nov 25, 2020

View reviewed changes

OrestChura reviewed Nov 30, 2020

View reviewed changes

modules/gapi/src/backends/onnx/gonnxbackend.cpp Outdated Show resolved Hide resolved

OrestChura reviewed Dec 1, 2020

View reviewed changes

modules/gapi/test/infer/gapi_infer_onnx_test.cpp Outdated Show resolved Hide resolved

OrestChura approved these changes Dec 11, 2020

View reviewed changes

alalek reviewed Dec 11, 2020

View reviewed changes

mpashchenkov force-pushed the mp/onnx-disable-output branch from bf4dba2 to 54c88f7 Compare February 11, 2021 09:22

TolyaTalamanov reviewed Feb 24, 2021

View reviewed changes

TolyaTalamanov requested changes Feb 25, 2021

View reviewed changes

modules/gapi/src/backends/onnx/gonnxbackend.cpp Show resolved Hide resolved

modules/gapi/src/backends/onnx/gonnxbackend.cpp Outdated Show resolved Hide resolved

TolyaTalamanov requested changes Feb 26, 2021

View reviewed changes

mpashchenkov assigned TolyaTalamanov Mar 2, 2021

TolyaTalamanov reviewed Mar 3, 2021

View reviewed changes

mpashchenkov force-pushed the mp/onnx-disable-output branch from e680faa to 4ae74e5 Compare March 5, 2021 10:00

TolyaTalamanov reviewed Mar 5, 2021

View reviewed changes

modules/gapi/test/infer/gapi_infer_onnx_test.cpp Outdated Show resolved Hide resolved

mpashchenkov force-pushed the mp/onnx-disable-output branch from 6407c02 to f88320d Compare March 5, 2021 14:33

TolyaTalamanov reviewed Mar 9, 2021

View reviewed changes

modules/gapi/test/infer/gapi_infer_onnx_test.cpp Outdated Show resolved Hide resolved

modules/gapi/test/infer/gapi_infer_onnx_test.cpp Outdated Show resolved Hide resolved

TolyaTalamanov reviewed Mar 16, 2021

View reviewed changes

modules/gapi/test/infer/gapi_infer_onnx_test.cpp Outdated Show resolved Hide resolved

modules/gapi/test/infer/gapi_infer_onnx_test.cpp Outdated Show resolved Hide resolved

TolyaTalamanov reviewed Mar 18, 2021

View reviewed changes

TolyaTalamanov approved these changes Mar 18, 2021

View reviewed changes

mpashchenkov added this to the 4.5.2 milestone Mar 29, 2021

dmatveev reviewed Mar 29, 2021

View reviewed changes

mpashchenkov force-pushed the mp/onnx-disable-output branch from 5214175 to a608ec7 Compare March 30, 2021 15:54

Added overload for cfgPostPros

61a4100

mpashchenkov force-pushed the mp/onnx-disable-output branch from a608ec7 to 61a4100 Compare March 31, 2021 09:21

dmatveev approved these changes Mar 31, 2021

View reviewed changes

alalek merged commit e11408f into opencv:master Mar 31, 2021

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #19885

Merged


		std::vector<bool> normalize;

		std::vector<std::string> d_out_names;

		ti.is_disabled = (std::find(params.d_out_names.begin(),
		params.d_out_names.end(), ti.name) != params.d_out_names.end()) && is_postproc;

Uh oh!

Conversation

mpashchenkov commented Nov 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Disable unnecessary outputs

How it works:

Tests:

Pull Request Readiness Checklist

Custom build:

Uh oh!

OrestChura left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

asmorkalov commented Dec 9, 2020

Uh oh!

OrestChura left a comment

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!

dmatveev commented Dec 14, 2020

Uh oh!

mpashchenkov commented Dec 15, 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!

Uh oh!

TolyaTalamanov commented Feb 24, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

mpashchenkov commented Mar 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mpashchenkov commented Nov 23, 2020 •

edited

Loading

OrestChura commented Dec 1, 2020 •

edited

Loading

mpashchenkov commented Dec 15, 2020 •

edited

Loading