Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Expand ie::Params to support config#18701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@TolyaTalamanov
Copy link
Contributor

@TolyaTalamanovTolyaTalamanov commentedOct 30, 2020•
edited by alalek
Loading

Pull Request Readiness Checklist

See details athttps://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 Macbuild_gapi_standalone:Linux x64=ade-0.1.1fbuild_gapi_standalone:Win64=ade-0.1.1fbuild_gapi_standalone:Mac=ade-0.1.1fbuild_gapi_standalone:Linux x64 Debug=ade-0.1.1fXbuild_image:Custom=centos:7Xbuildworker:Custom=linux-1build_gapi_standalone:Custom=ade-0.1.1fbuild_image:Custom=ubuntu-openvino-2020.3.0:16.04build_image:Custom Win=openvino-2021.1.0build_image:Custom Mac=openvino-2021.1.0test_modules:Custom=gapitest_modules:Custom Win=gapitest_modules:Custom Mac=gapibuildworker:Custom=linux-1# disabled due high memory usage: test_opencl:Custom=ONtest_opencl:Custom=OFFtest_bigdata:Custom=1test_filter:Custom=*

@TolyaTalamanov
Copy link
ContributorAuthor

@dmatveev

@TolyaTalamanovTolyaTalamanovforce-pushed theat/introduce-config-for-ie-params branch 2 times, most recently fromb71d343 to45c86e5CompareNovember 2, 2020 09:12
@TolyaTalamanov
Copy link
ContributorAuthor

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

@dmatveevdmatveev added this to the4.5.1 milestoneNov 2, 2020
@dmatveev
Copy link
Contributor

@rgarnov@anton-potapov please prioritize

Copy link
Contributor

@rgarnovrgarnov left a comment

Choose a reason for hiding this comment

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

Looks good

model_path,
weights_path,
device_id,
{{"unsupported_config","some_value"}}};
Copy link
Contributor

Choose a reason for hiding this comment

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

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")

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

@rgarnovrgarnovNov 2, 2020•
edited
Loading

Choose a reason for hiding this comment

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

I propose to change the method name tosetCfg or something similar.cfgPlugin looks like it expects some plugin name at input and may be misleading. In its turnsetCfg can conflict withcfgInputs/Outputs though since there are different meanings ofcfg in these names

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm... exceptconstInput(). :)

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

Copy link
Member

Choose a reason for hiding this comment

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

setPluginConfig() andgetPluginConfig() (optional, but can be used in tests)

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Ok, rename topluginConfig

@TolyaTalamanovTolyaTalamanovforce-pushed theat/introduce-config-for-ie-params branch from45c86e5 to73d5a6fCompareNovember 2, 2020 14:15
@TolyaTalamanovTolyaTalamanovforce-pushed theat/introduce-config-for-ie-params branch from73d5a6f tob9604daCompareNovember 2, 2020 14:16
auto gender = outputs.at("prob");
cv::GComputationcomp(cv::GIn(in),cv::GOut(age, gender));

auto pp = cv::gapi::ie::Params<cv::gapi::Generic>{"age-gender-generic",
Copy link
Contributor

Choose a reason for hiding this comment

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

would not this test should be extended to test generic version ofParams ?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

It is alreadygeneric, is it ?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Added tests onnon-generic

Comment on lines +427 to +430
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))));
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW is it possible to create positive test ?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

For which option, for example ?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Added test

@TolyaTalamanov
Copy link
ContributorAuthor

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

@TolyaTalamanov
Copy link
ContributorAuthor

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

@dmatveev
Copy link
Contributor

Can it be merged today ?

Approved, but@alalek is who merges.:)


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

@alalekalalekNov 3, 2020•
edited
Loading

Choose a reason for hiding this comment

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

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 isi7-6700K.

Copy link
Member

Choose a reason for hiding this comment

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

Caused by oldbuild_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).

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks! I'll replace option asap

@alalekalalek merged commit2a3cdba intoopencv:masterNov 3, 2020
@dmatveev
Copy link
Contributor

@TolyaTalamanov can you please update our wiki, too?

@alalekalalek mentioned this pull requestNov 27, 2020
@dmatveevdmatveev mentioned this pull requestJun 3, 2021
6 tasks
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull requestMar 30, 2023
…ig-for-ie-paramsExpand 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
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@dmatveevdmatveevdmatveev approved these changes

@anton-potapovanton-potapovanton-potapov left review comments

@alalekalalekalalek left review comments

@rgarnovrgarnovrgarnov approved these changes

Assignees

@dmatveevdmatveev

Projects

None yet

Milestone

4.5.1

Development

Successfully merging this pull request may close these issues.

5 participants

@TolyaTalamanov@dmatveev@anton-potapov@alalek@rgarnov
[8]ページ先頭
©2009-2025 Movatter.jp