G-API: Support vaargs for cv.compile_args by TolyaTalamanov · Pull Request #20196 · opencv/opencv

TolyaTalamanov · 2021-06-02T07:52:20Z

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

Build configuration

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-2021.3.0:20.04
build_image:Custom Win=openvino-2021.2.0
build_image:Custom Mac=openvino-2021.2.0

test_modules:Custom=gapi,python2,python3,java
test_modules:Custom Win=gapi,python2,python3,java
test_modules:Custom Mac=gapi,python2,python3,java

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 · 2021-06-02T08:12:46Z

@dmatveev @mpashchenkov @sivanov-work Folks, have a look, please

TolyaTalamanov · 2021-06-02T10:31:26Z

@alalek Compatibility with python2 seems to be broken, can we exclude python2 since it's already deprecated ?

TolyaTalamanov · 2021-06-02T10:34:22Z

modules/gapi/misc/python/shadow_gapi.hpp

-   GAPI_EXPORTS_W GCompileArgs compile_args(gapi::GNetPackage pkg);
-   GAPI_EXPORTS_W GCompileArgs compile_args(gapi::GKernelPackage kernels, gapi::GNetPackage nets);
+   struct GAPI_EXPORTS_W_SIMPLE GCompileArg {
+       GAPI_WRAP GCompileArg(gapi::GKernelPackage pkg);


To extend number of possible types from another module need to do:

// shadow_somemodule.hpp struct GCompileArg { GAPI_WRAP GCompileArg(module::SomeArg arg); };

This code adds new overload to GCompileArg ctor

But how did you solve variadic arguments passing?

it's solved indirectly through python's compile_args (which probably requires a better PR name since it's confusing - vaargs is a thing in C++ as well).

map() calls a function for each element of a range. given that the function is a GCompileArg ctor, we convert "raw" args into a range containing cv2.GCompileArg(arg) elements (by the way, does ctor qualify as a function?)

by the way, does ctor qualify as a function

Why not ? https://stackoverflow.com/questions/11943831/python-how-to-put-constructors-in-map-function

andrey-golubev · 2021-06-05T19:31:22Z

modules/gapi/misc/python/shadow_gapi.hpp

-   GAPI_EXPORTS_W GCompileArgs compile_args(gapi::GNetPackage pkg);
-   GAPI_EXPORTS_W GCompileArgs compile_args(gapi::GKernelPackage kernels, gapi::GNetPackage nets);
+   struct GAPI_EXPORTS_W_SIMPLE GCompileArg {
+       GAPI_WRAP GCompileArg(gapi::GKernelPackage pkg);


it's solved indirectly through python's compile_args (which probably requires a better PR name since it's confusing - vaargs is a thing in C++ as well).

map() calls a function for each element of a range. given that the function is a GCompileArg ctor, we convert "raw" args into a range containing cv2.GCompileArg(arg) elements (by the way, does ctor qualify as a function?)

andrey-golubev · 2021-06-05T19:39:48Z

modules/gapi/misc/python/shadow_gapi.hpp

+   struct GAPI_EXPORTS_W_SIMPLE GCompileArg {
+       GAPI_WRAP GCompileArg(gapi::GKernelPackage pkg);
+       GAPI_WRAP GCompileArg(gapi::GNetPackage pkg);
+   };


while I like the idea, this API is not quite equivalent. what if, as a user, I have my own gapi::GMyOwnPackage - how can I use compile_args machinery to wrap this external-to-G-API package?
it seems remotely possible with the old API (just add a new overload of compile_args) - albeit, I'm not sure if python has anything like function overload resolution/adl (wondering how this old API works) - if this is out of scope for this (e.g. G-API only supports packages that it provides itself via compile_args and users have to do something else), then feel free to ignore this.

Wow, glad to see u :)

As for gapi::GMyOwnPackage what do u mean ? Do u want to use your own compile argument ?
From python user can't define new compile argument.

Wow, glad to see u :)

Just occasionally looking at things here, nothing too serious 😉

As for gapi::GMyOwnPackage what do u mean ? Do u want to use your own compile argument ?

Yes, imagine such a use case (maybe someone wants their own package - it is the thing that provides the kernels/etc, right? can't remember anymore).

From python user can't define new compile argument.

Well, it should be doable from C++ and then C++ gets magically converted into python - I guess, ideally, G-API should be convenient to use from python as well, so if someone supplies their own package, it could be used from python API as well.
However, this is a purely artificial use case. I wrote this comment in the meaning of:
you change the API and this (might) limit the usability - e.g. the ability to use custom packages in python. if this was never a concern, then this change is perfectly fine.

(glad to see you as well!)

Yes, imagine such a use case (maybe someone wants their own package - it is the thing that provides the kernels/etc, right? can't remember anymore).

Again, what's the purpose of the gapi::GMyOwnPackage ? First of all it won't be recognized inside G-API, right ? Secondly if the user want to pass his own set of kernels it can be easily done by using cv.gapi.kernels(....) which returns gapi::GKernelPackage

I mean that user defined compile arguments are useless because there is no support inside G-API to handle them.

ah, I get it now. no objections then.

…port-vaargs-compile-args

modules/gapi/misc/python/test/test_gapi_core.py

mpashchenkov · 2021-06-15T14:55:51Z

modules/gapi/misc/python/test/test_gapi_core.py

+            self.assertEqual(centers.shape[1], sz[1]);
+            self.assertEqual(centers.shape[0], K);
+            self.assertTrue(centers.size != 0);


modules/gapi/misc/python/test/test_gapi_core.py

modules/gapi/misc/python/test/test_gapi_streaming.py

modules/gapi/misc/python/test/test_gapi_core.py

TolyaTalamanov · 2021-06-16T06:16:47Z

@alalek Can it be merged ?

alalek · 2021-06-16T14:26:54Z

modules/gapi/misc/python/package/gapi/__init__.py



+@register('cv2')
+def compile_args(*args):


cv2 => cv2.gapi ?

(as there is no "G" prefix)

but in c++ it's a part of cv namespace, not cv::gapi

It will be a bit weird to keep it in cv.gapi package in that case. Are u Ok to leave it as is ?

@dmatveev Please confirm this usage.

@alalek yes somehow we have cv::compile_args but not cv::gapi::compile_args in our C++ API.

It is definitely worth aligning in 5.0

Then it makes sense to keep it in cv.gapi for python.

TolyaTalamanov · 2021-06-18T15:42:48Z

@alalek

…-compile-args G-API: Support vaargs for cv.compile_args * Support cv.compile_args to work with variadic number of inputs * Disable python2.x G-API * Move compile_args to gapi pkg

Support cv.compile_args to work with variadic number of inputs

be2303d

TolyaTalamanov requested a review from dmatveev June 2, 2021 07:52

TolyaTalamanov added category: g-api / gapi category: python bindings labels Jun 2, 2021

TolyaTalamanov requested review from mpashchenkov and sivanov-work June 2, 2021 07:52

TolyaTalamanov commented Jun 2, 2021

View reviewed changes

andrey-golubev reviewed Jun 5, 2021

View reviewed changes

Merge branch 'master' of https://github.com/opencv/opencv into at/sup…

c6d99df

…port-vaargs-compile-args

mpashchenkov reviewed Jun 15, 2021

View reviewed changes

modules/gapi/misc/python/test/test_gapi_core.py Outdated Show resolved Hide resolved

mpashchenkov approved these changes Jun 15, 2021

View reviewed changes

Disable python2.x G-API

ca3787d

TolyaTalamanov force-pushed the at/support-vaargs-compile-args branch from 6f8d6e7 to ca3787d Compare June 15, 2021 15:07

alalek reviewed Jun 16, 2021

View reviewed changes

Move compile_args to gapi pkg

adb2faa

alalek assigned mpashchenkov Jun 18, 2021

alalek merged commit 53eca2f into opencv:master Jun 18, 2021

alalek mentioned this pull request Oct 15, 2021

(5.x) Merge 4.x #20886

Merged



		@register('cv2')
		def compile_args(*args):

Uh oh!

Conversation

TolyaTalamanov commented Jun 2, 2021

Pull Request Readiness Checklist

Build configuration

Uh oh!

TolyaTalamanov commented Jun 2, 2021

Uh oh!

TolyaTalamanov commented Jun 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TolyaTalamanov Jun 2, 2021 • 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!

TolyaTalamanov Jun 2, 2021 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrey-golubev Jun 6, 2021 • 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!

andrey-golubev Jun 8, 2021 • 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TolyaTalamanov commented Jun 16, 2021

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!

TolyaTalamanov commented Jun 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

TolyaTalamanov commented Jun 2, 2021 •

edited

Loading

TolyaTalamanov Jun 2, 2021 •

edited

Loading

TolyaTalamanov Jun 2, 2021 •

edited

Loading

andrey-golubev Jun 6, 2021 •

edited

Loading

andrey-golubev Jun 8, 2021 •

edited

Loading