Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork56.4k
add REDUCE_SUM2#13879
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
add REDUCE_SUM2#13879
Uh oh!
There was an error while loading.Please reload this page.
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
chacha21 commentedFeb 21, 2019
Can anyone help me understand the build failure related to perf_test ? I do not understand how I am supposed to fix that. |
| elseif( op == CV_REDUCE_SUM2 ) | ||
| { | ||
| if(sdepth == CV_8U && ddepth == CV_32S) | ||
| func =GET_OPTIMIZED(reduceSum2R8u32s); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
RemoveGET_OPTIMIZED().
There is no corresponding function anyway.
Also changes in "ReduceFunc" broke all corresponding calls (changes can be reverted, see corresponding comment).
| template<typename T,typename ST,classOp>staticvoid | ||
| reduceR_(const Mat& srcmat, Mat& dstmat) | ||
| reduceR_(const Mat& srcmat, Mat& dstmat,bool applyOpOnFirstRow =false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Good idea, I am on it
| #defineCV_REDUCE_AVG 1 | ||
| #defineCV_REDUCE_MAX 2 | ||
| #defineCV_REDUCE_MIN 3 | ||
| #defineCV_REDUCE_SUM2 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please don't touch ".c" files anymore (deprecated).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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)
chacha21 commentedFeb 22, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I am sorry, I have read the doc, but I stil don't know how to understand the perf_test failure : |
alalek commentedFeb 22, 2019
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") |
chacha21 commentedFeb 24, 2019
Once the "perf_test" problem will be resolved, I plan to add REDUCE_SUMABS to compute the absolute sum of elements. |
chacha21 commentedFeb 25, 2019
I have followed the instructions ofhttps://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 :
|
alalek commentedFeb 25, 2019
This is completely different XML (Google Test output). You need to specify |
chacha21 commentedFeb 25, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I did not forget, neither |
alalek commentedFeb 25, 2019
|
chacha21 commentedFeb 28, 2019
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 : |
chacha21 commentedMay 4, 2019
I am sorry, but I am still unable to provide the perf test, I still don't understand how to do that. |
chacha21 commentedDec 18, 2019
I dont understand the buildbot error " No regression data for dst argument" |
alalek commentedDec 18, 2019
Performance tests has some accuracy checks:
|
added SANITY_CHECK_NOTHING() under REDUCE_SUM2
asmorkalov commentedJan 31, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Remaining item for the PR:
|
asenyaev commentedApr 7, 2021
jenkins cn please retry a build |
mshabunin commentedApr 18, 2023
@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 left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍
mshabunin commentedMay 1, 2023
@asmorkalov , thank you! |
add REDUCE_SUM2opencv#13879 proposal to add REDUCE_SUM2 to cv::reduce, an operation that sums up the square of elements
add REDUCE_SUM2opencv#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