Expand ie::Params to support config #18701

TolyaTalamanov · 2020-10-30T11:49:16Z

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

force_builders=Custom,Custom Win,Custom Mac
build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f
Xbuild_image:Custom=centos:7
Xbuildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f
build_image:Custom=ubuntu-openvino-2020.3.0:16.04
build_image:Custom Win=openvino-2021.1.0
build_image:Custom Mac=openvino-2021.1.0
test_modules:Custom=gapi
test_modules:Custom Win=gapi
test_modules:Custom Mac=gapi
buildworker:Custom=linux-1
# disabled due high memory usage: test_opencl:Custom=ON
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

TolyaTalamanov · 2020-10-30T13:38:55Z

@dmatveev

TolyaTalamanov · 2020-11-02T10:05:19Z

@rgarnov @anton-potapov Could you have a look, please ?

dmatveev · 2020-11-02T13:12:26Z

@rgarnov @anton-potapov please prioritize

rgarnov

Looks good

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

dmatveev · 2020-11-02T13:43:47Z

modules/gapi/test/infer/gapi_infer_ie_test.cpp

+                                                model_path,
+                                                weights_path,
+                                                device_id,
+                                                {{"unsupported_config", "some_value"}}};


No. Putting everything to a cosntructor bloats up everything.

I'd expect to see

pp.cfgPlugin(map);

or even

pp .cfgPlugin("key", "value") .cfgPlugin("key", "value")

I propose to change the method name to setCfg or something similar. cfgPlugin looks like it expects some plugin name at input and may be misleading. In its turn setCfg can conflict with cfgInputs/Outputs though since there are different meanings of cfg in these names

we have all other methods starting with cfg, and via this you configure plugin, though I agree it doesn't look very good

hmm... except constInput(). :)

Maybe .pluginConfig is what we need - but it can always be added atop.

setPluginConfig() and getPluginConfig() (optional, but can be used in tests)

Ok, rename to pluginConfig

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

anton-potapov · 2020-11-03T09:34:25Z

modules/gapi/test/infer/gapi_infer_ie_test.cpp

+    auto gender  = outputs.at("prob");
+    cv::GComputation comp(cv::GIn(in), cv::GOut(age, gender));
+
+    auto pp = cv::gapi::ie::Params<cv::gapi::Generic>{"age-gender-generic",


would not this test should be extended to test generic version of Params ?

It is already generic, is it ?

Added tests on non-generic

anton-potapov · 2020-11-03T09:35:08Z

modules/gapi/test/infer/gapi_infer_ie_test.cpp

+                                                       device_id}.pluginConfig({{"unsupported_config", "some_value"}});
+
+    EXPECT_ANY_THROW(comp.compile(cv::GMatDesc{CV_8U,3,cv::Size{320, 240}},
+                     cv::compile_args(cv::gapi::networks(pp))));


BTW is it possible to create positive test ?

For which option, for example ?

TolyaTalamanov · 2020-11-03T11:42:02Z

@anton-potapov Could you have a look, please ?

TolyaTalamanov · 2020-11-03T12:53:14Z

@dmatveev @rgarnov @anton-potapov Can it be merged today ?

dmatveev · 2020-11-03T15:27:18Z

Can it be merged today ?

Approved, but @alalek is who merges.:)

alalek · 2020-11-03T17:10:58Z

modules/gapi/test/infer/gapi_infer_ie_test.cpp

+
+    auto pp = cv::gapi::ie::Params<AgeGender> {
+        model_path, weights_path, device_id
+    }.cfgOutputLayers({ "age_conv3", "prob" }).pluginConfig({{"ENFORCE_BF16", "NO"}});


Build:

[ FAILED ] 2 tests, listed below: [ FAILED ] TestAgeGenderIE.CPUConfigGeneric [ FAILED ] TestAgeGenderIE.CPUConfig

with message

Exception message: [NOT_FOUND] Unsupported property ENFORCE_BF16 by CPU plugin

CPU is i7-6700K.

Caused by old build_image:Custom Win=openvino-2020.3.0.

It is OpenVINO LTS version so it is better to add handling for that (replace property or skip test).

Thanks! I'll replace option asap

dmatveev · 2020-11-04T20:41:10Z

@TolyaTalamanov can you please update our wiki, too?

…ig-for-ie-params Expand ie::Params to support config * Add config to IE params * Add test * Remove comments from tests * Rename to pluginConfig * Add one more overloads for pluginConfig * Add more tests

TolyaTalamanov added 2 commits October 30, 2020 14:42

Add config to IE params

f06c8dd

Add test

8360377

TolyaTalamanov force-pushed the at/introduce-config-for-ie-params branch 2 times, most recently from b71d343 to 45c86e5 Compare November 2, 2020 09:12

dmatveev added this to the 4.5.1 milestone Nov 2, 2020

rgarnov approved these changes Nov 2, 2020

View reviewed changes

dmatveev requested changes Nov 2, 2020

View reviewed changes

TolyaTalamanov force-pushed the at/introduce-config-for-ie-params branch from 45c86e5 to 73d5a6f Compare November 2, 2020 14:15

Remove comments from tests

b9604da

TolyaTalamanov force-pushed the at/introduce-config-for-ie-params branch from 73d5a6f to b9604da Compare November 2, 2020 14:16

dmatveev approved these changes Nov 2, 2020

View reviewed changes

Rename to pluginConfig

47fb85a

anton-potapov reviewed Nov 3, 2020

View reviewed changes

modules/gapi/include/opencv2/gapi/infer/ie.hpp Outdated Show resolved Hide resolved

Add one more overloads for pluginConfig

874acc2

anton-potapov reviewed Nov 3, 2020

View reviewed changes

Add more tests

245c168

alalek assigned dmatveev Nov 3, 2020

alalek reviewed Nov 3, 2020

View reviewed changes

alalek merged commit 2a3cdba into opencv:master Nov 3, 2020

alalek mentioned this pull request Nov 27, 2020

Merge 4.x #18951

Merged

dmatveev mentioned this pull request Jun 3, 2021

G-API: Support remote inference #20156

Merged

6 tasks

Uh oh!

Expand ie::Params to support config #18701

Expand ie::Params to support config #18701

Uh oh!

Conversation

TolyaTalamanov commented Oct 30, 2020 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

TolyaTalamanov commented Oct 30, 2020

Uh oh!

TolyaTalamanov commented Nov 2, 2020

Uh oh!

dmatveev commented Nov 2, 2020

Uh oh!

rgarnov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rgarnov Nov 2, 2020 • 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!

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!

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!

TolyaTalamanov commented Nov 3, 2020

Uh oh!

TolyaTalamanov commented Nov 3, 2020

Uh oh!

dmatveev commented Nov 3, 2020

Uh oh!

alalek Nov 3, 2020 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmatveev commented Nov 4, 2020

Uh oh!

Uh oh!

TolyaTalamanov commented Oct 30, 2020 •

edited by alalek

Loading

rgarnov Nov 2, 2020 •

edited

Loading

alalek Nov 3, 2020 •

edited

Loading