add REDUCE_SUM2 #13879
add REDUCE_SUM2 #13879
Conversation
proposal to add REDUCE_SUM2 to cv::reduce, an operation that sums up the square of elements
put AutoBuffer outside the main reduceC_ loop
Added SUM_REDUCE2 to tests
fixed test of REDUCE_SUM2
|
Can anyone help me understand the build failure related to perf_test ? I do not understand how I am supposed to fix that. |
| else if( op == CV_REDUCE_SUM2 ) | ||
| { | ||
| if(sdepth == CV_8U && ddepth == CV_32S) | ||
| func = GET_OPTIMIZED(reduceSum2R8u32s); |
There was a problem hiding this comment.
Remove GET_OPTIMIZED().
There is no corresponding function anyway.
Also changes in "ReduceFunc" broke all corresponding calls (changes can be reverted, see corresponding comment).
| @@ -605,7 +605,7 @@ namespace cv | |||
| { | |||
|
|
|||
| template<typename T, typename ST, class Op> static void | |||
| reduceR_( const Mat& srcmat, Mat& dstmat ) | |||
| reduceR_( const Mat& srcmat, Mat& dstmat, bool applyOpOnFirstRow = false) | |||
There was a problem hiding this comment.
Implementation changes doesn't look good.
What if you want to calculate product instead of sum? "0" value doesnt' work anymore
IMHO, It is better to define separate "Init" functor:
- NoOp for old cases
- new SqValue
-buf[i] = src[i];
+buf[i] = initOp(src[i]);There was a problem hiding this comment.
Good idea, I am on it
| @@ -1490,6 +1490,7 @@ CVAPI(void) cvNormalize( const CvArr* src, CvArr* dst, | |||
| #define CV_REDUCE_AVG 1 | |||
| #define CV_REDUCE_MAX 2 | |||
| #define CV_REDUCE_MIN 3 | |||
| #define CV_REDUCE_SUM2 4 | |||
There was a problem hiding this comment.
Please don't touch ".c" files anymore (deprecated).
There was a problem hiding this comment.
Ok, but in that case I will have to change all references to CV_REDUCE_* in cpp code, in favor of cv::REDUCE_*
There was a problem hiding this comment.
Ok, please use enum values: REDUCE_ (cv:: prefix is not needed)
+prefer cv::REDUCE_* instead of CV_REDUCE throughout the code, so that core_c.h remains untouched and doesn't know about CV_REDUCE_SUM2 +get rid of GET_OPTIMIZED() in reduceR and reduceC +add an additional Operator template functor to reduceR and reduceC that is responsible for the very first value (by default just copies the value, and will square it for REDUCE_SUM2)
|
I am sorry, I have read the doc, but I stil don't know how to understand the perf_test failure : |
You can check it locally by passing "--perf_verify_sanity" argument. https://github.com/opencv/opencv/wiki/HowToUsePerfTests#how-to-update-perf-data Need to update test data and prepare PR into opencv_extra (with the same branch name as this PR: "REDUCE_SUM2") |
|
Once the "perf_test" problem will be resolved, I plan to add REDUCE_SUMABS to compute the absolute sum of elements. |
|
I have followed the instructions of https://github.com/opencv/opencv/wiki/HowToUsePerfTests#how-to-update-perf-data, but I get a totally different xml that the original core.xml Here is the beginning of my output :
|
|
This is completely different XML (Google Test output). You need to specify |
|
I did not forget, neither |
|
|
I did not use --gtest_output at all, I just ran the python scripts as mentioned in HowToUsePerfTests. The xml I get in the generated core-[date].xml file is the one state above, very different from the expected There must be something I misunderstood. Here is a recap : |
|
I am sorry, but I am still unable to provide the perf test, I still don't understand how to do that. |
|
I dont understand the buildbot error " No regression data for dst argument" |
|
Performance tests has some accuracy checks:
|
added SANITY_CHECK_NOTHING() under REDUCE_SUM2
asmorkalov
added
category: core
Hackathon
|
Remaining item for the PR:
|
|
jenkins cn please retry a build |
|
@asmorkalov , I've updated the PR:
Below are my performance results for 1, 4 and 8 threads. There is a slowdown when reduce is performed along 0th dimension for smaller image sizes when number of threads is more than 1.
|
|
@asmorkalov , thank you! |
add REDUCE_SUM2 opencv#13879 proposal to add REDUCE_SUM2 to cv::reduce, an operation that sums up the square of elements
add REDUCE_SUM2 opencv#13879 proposal to add REDUCE_SUM2 to cv::reduce, an operation that sums up the square of elements
proposal to add REDUCE_SUM2 to cv::reduce, an operation that sums up the square of elements