G-API: Add transpose operation #20107

sivanov-work · 2021-05-17T14:48:18Z

Add transpose operation to gapi which is similar with opencv transpose https://docs.opencv.org/master/d2/de8/group__core__array.html#ga46630ed6c0ea6254a35f447289bd7404

Add Unit/Perf test

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-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=*

aDanPin · 2021-05-18T11:33:08Z

modules/gapi/include/opencv2/gapi/core.hpp

@@ -575,6 +575,12 @@ namespace core {
            return std::make_tuple(empty_gopaque_desc(), empty_array_desc(), empty_array_desc());
        }
    };
+
+    G_TYPED_KERNEL(GTranspose, <GMat(GMat)>, "org.opencv.core.transpose") {


Should it be 'org.opencv.core.matrixop.transpose' or 'org.opencv.core.transform.transpose' ?

I believe the string name is correct as it is not used elsewhere;

What I'd recommend to use is G_API_OP instead of the G_TYPED_KERNEL.

(...yes our code uses the old macro almost everywhere)

@aDanPin , @dmatveev i've rechecked string name and looks like it is unique
@dmatveev Ok, will rename macro

Shall i rename all old-styled names in this file additionally?

aDanPin · 2021-05-18T11:40:56Z

modules/gapi/perf/common/gapi_core_perf_tests_inl.hpp

+    std::tie(cmpF, sz_in, type, compile_args) = GetParam();
+
+    initMatrixRandU(type, sz_in, type, false);
+    cv::Mat out_transpose_gapi, out_transpose_ocv;


There is out_mat_gapi and out_mat_ocv already
I think you cold use it instead out_transpose_gapi and out_transpose_ocv next time

dmatveev

Good!

dmatveev · 2021-05-19T17:48:35Z

modules/gapi/test/common/gapi_tests_common.hpp

@@ -542,6 +542,7 @@ struct TestWithParamsSpecific : public TestWithParamsBase<ParamsSpecific<Specifi
 * @param ...       list of names of user-defined parameters. if there are no parameters, the list
 *                  must be empty.
 */
+ //TODO: Consider to remove `Number` and use `std::tuple_size<decltype(std::make_tuple(__VA_ARGS__))>::value`


@andrey-golubev any thoughts on this? :)

oh, looking at it in a silo, that would actually be nice. std::make_tuple is constexpr since C++14 though (is OpenCV on it already?), so in C++11 that won't help.
there's BOOST_PP_VARIADIC_SIZE by the way (which is surprisingly interesting) that gives size(__VA_ARGS__) during preprocessing and might help here as well (but I don't remember this code anymore).

oh, i missed that tuple is constexpr in C++14

@andrey-golubev What do you think about the follow home-grown solution?

#include <iostream> #include <string> template<class ...T> constexpr size_t args_count(T&&...args) { return sizeof...(args); } #define VAARG_NUM(...) args_count(__VA_ARGS__) int main(int argc, char **argv) { int a,b; double d; char c; std::string str; struct Foo {}; static_assert(VAARG_NUM() == 0, "Args pack 0 failed, expected 0"); static_assert(VAARG_NUM(str) == 1, "Args pack 1 failed, expected 1"); static_assert(VAARG_NUM(a,b,d,c,str) == 5, "Args pack 2 failed, expected 5"); static_assert(VAARG_NUM(1, "c", 1.0,Foo {}) == 4, "Args pack 3 failed, expected 4");

it depends on whether __VA_ARGS__ are variables or types (which is also a problem with std::make_tuple() I think). you'd need a type function in the latter case.
also note that the logic is to "##" the number to a macro, so that it becomes a new macro name e.g.:
Number = 0: use DEFINE_SPECIFIC_PARAMS_0
Number = 5: use DEFINE_SPECIFIC_PARAMS_5
if I remember this correctly.
so I don't think you'd be able to pull that off with a (possibly-a-type) function to be honest, since the compiler is invoked after the preprocessor.

anyhow, at least you'd need to wrap existing macro into one more to get the literal before using it, which is of course might also be problematic :)

in theory, the whole thing could be rewritten to only use templates (making macro a no-op), not sure why I didn't go this way. there was something about declaring member variables which couldn't be done with a simple template magic (maybe with a complicated magic, but it seemed too much at the time probably).

it depends on whether VA_ARGS are variables or types

I think I understand your point: yes - my home-grown function implementation is for variables only, to cover the types we need another solution. But implementation of generic solution (both for types & variables) requires a lot of effort in decomposition sub-problems for types and variables in single overwhelmed entity.

but for this specific case we receives variables only (for checking fixture correctness), and looks like it is enough to work over __VA_ARGS__ as variables here

Sorry, if i understood you in a wrong way

as I look at it more and more, seems like __VA_ARGS__ are actually names of to-be-later-created-variables.
you can look at what DEFINE_SPECIFIC_PARAMS does to get the idea.

anyhow, if you figure out a nice way to simplify this, by all means feel free to update this code.

dmatveev · 2021-05-19T17:49:16Z

modules/gapi/include/opencv2/gapi/core.hpp

@@ -575,6 +575,12 @@ namespace core {
            return std::make_tuple(empty_gopaque_desc(), empty_array_desc(), empty_array_desc());
        }
    };
+
+    G_TYPED_KERNEL(GTranspose, <GMat(GMat)>, "org.opencv.core.transpose") {


I believe the string name is correct as it is not used elsewhere;

What I'd recommend to use is G_API_OP instead of the G_TYPED_KERNEL.

dmatveev · 2021-05-19T17:49:36Z

modules/gapi/include/opencv2/gapi/core.hpp

@@ -575,6 +575,12 @@ namespace core {
            return std::make_tuple(empty_gopaque_desc(), empty_array_desc(), empty_array_desc());
        }
    };
+
+    G_TYPED_KERNEL(GTranspose, <GMat(GMat)>, "org.opencv.core.transpose") {


(...yes our code uses the old macro almost everywhere)

dmatveev · 2021-05-19T17:50:36Z

modules/gapi/perf/common/gapi_core_perf_tests_inl.hpp

+    std::tie(cmpF, sz_in, type, compile_args) = GetParam();
+
+    initMatrixRandU(type, sz_in, type, false);
+    cv::Mat out_transpose_gapi, out_transpose_ocv;


dmatveev · 2021-05-19T17:51:13Z

modules/gapi/perf/cpu/gapi_core_perf_tests_cpu.cpp

+INSTANTIATE_TEST_CASE_P(TransposePerfTestCPU, TransposePerfTest,
+                        Combine(Values(AbsExact().to_compare_f()),
+                                Values(szSmall128, szVGA, sz720p, sz1080p),
+                                Values(CV_8UC1, CV_16UC1, CV_16SC1, CV_32FC1),


Are three-channel images supported? I believe it is the case for MTCNN (BGR), @dbudniko please correct me.

i think i'll check it

i've added additional types to param set and it still works OK

aDanPin

LGTM!

sivanov-work · 2021-05-24T15:02:05Z

@alalek
Hi could you merge this PR please? it looks like all relates stuffs are applied

G-API: Add transpose operation * Add kernels decl & def * Add draft for UT * Fix UT for Transpose * Add perf test * Fix docs * Apply comments

sivanov-work added 4 commits May 7, 2021 13:46

Add kernels decl & def

d601e2f

Add draft for UT

d9be42c

Fix UT for Transpose

d02886e

Add perf test

fc48fec

sivanov-work added the category: g-api / gapi label May 17, 2021

Fix docs

1330b54

sivanov-work requested review from dmatveev, OrestChura and rgarnov May 18, 2021 08:59

aDanPin reviewed May 19, 2021

View reviewed changes

dmatveev approved these changes May 19, 2021

View reviewed changes

dmatveev self-assigned this May 19, 2021

dmatveev added this to the 4.5.3 milestone May 19, 2021

Apply comments

a3d1cc5

aDanPin approved these changes May 20, 2021

View reviewed changes

alalek merged commit 2b06208 into opencv:master May 25, 2021

alalek mentioned this pull request Jun 4, 2021

(5.x) Merge 4.x #20216

Merged

sivanov-work deleted the gapi_transpose_op branch March 5, 2022 05:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

G-API: Add transpose operation #20107

G-API: Add transpose operation #20107

sivanov-work commented May 17, 2021 •

edited

Loading

aDanPin May 18, 2021

dmatveev May 19, 2021

dmatveev May 19, 2021

sivanov-work May 20, 2021

sivanov-work May 20, 2021

aDanPin May 18, 2021

dmatveev May 19, 2021

sivanov-work May 20, 2021

dmatveev left a comment

dmatveev May 19, 2021

andrey-golubev May 19, 2021 •

edited

Loading

sivanov-work May 20, 2021

andrey-golubev May 20, 2021 •

edited

Loading

andrey-golubev May 20, 2021

sivanov-work May 20, 2021

andrey-golubev May 20, 2021

dmatveev May 19, 2021

dmatveev May 19, 2021

dmatveev May 19, 2021

dmatveev May 19, 2021

sivanov-work May 20, 2021

sivanov-work May 20, 2021

aDanPin left a comment

sivanov-work commented May 24, 2021

G-API: Add transpose operation #20107

G-API: Add transpose operation #20107

Conversation

sivanov-work commented May 17, 2021 • edited Loading

Pull Request Readiness Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmatveev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrey-golubev May 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrey-golubev May 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aDanPin left a comment

Choose a reason for hiding this comment

sivanov-work commented May 24, 2021

sivanov-work commented May 17, 2021 •

edited

Loading

andrey-golubev May 19, 2021 •

edited

Loading

andrey-golubev May 20, 2021 •

edited

Loading