Fix pyrlk.cl's wrong output of calcOpticalFlowPyrLK function's output vect…#8609
Conversation
| eq = std::abs(cpuErr[i] - err[i]) < 0.01; | ||
| if(!eq && | ||
| winSize.width >= 16 && winSize.height >=16 && | ||
| winSize.width <=24 && winSize.height <=24) |
There was a problem hiding this comment.
This test is intended to test optical flow implementation (any "optimized" version with passed parameters).
So this winSize limitation looks illogical here.
Especially, these winSize values are not covered by test parameters below
BTW, Please ignore "OCL_" prefix here - it is for legacy reasons here (to compare with OpenCV 2.4 tests). It doesn't force OpenCL code.
There was a problem hiding this comment.
Hi alalek,
I totally get your point. This test not only works for OpenCL, but also works for the CUDA and any other optimized version code. Therefore, this modification may badly influence the relevant code using this test.
In modules/video/src/opencl/pyrlk.cl file, however, the kernel can only support the winSize within 16*16 to 24*24, although its input accepts the winSize outside this range. In this kernel, the input winSize bigger than 24*24 will be referred as 24*24, the input winSize smaller than 16*16 will be referred as 16*16. In the test case 2 and 3, with winSize 25*25, although the kernel can output the correct result of nextPts, the output of err is wrong even with the patch I pulled. This is the reason why I modify this test adding the winSize limitation in it.
So, are there any ways to resolve this problem? How about that I add a new test specifically for OpenCL optical flow? Or would you mind that give me any other ideas?
Best,
There was a problem hiding this comment.
This test not only works for OpenCL, but also works for the CUDA and any other optimized version code.
CUDA has own API.
Available code path for this function is: CPU, CPU optimized with intrinsic (SSE/NEON), OpenCL, OpenVX (disabled due accuracy).
How about that I add a new test specifically for OpenCL
No, you should not do this. At least you can't force OpenCL code path, especially if there is no OpenCL device or something is not supported (like ocl::Image2D::isFormatSupported(CV_32F, 1, false)).
This test is intended to check function output. It is launched on many platforms and we check that result is valid on all of them.
In this kernel, the input winSize bigger than 2424 will be referred as 2424, the input winSize smaller than 1616 will be referred as 1616.
If OpenCL kernel doesn't support some winSize values (or other parameters)), then "host" code should check for these cases and bailout to other implementations.
There was a problem hiding this comment.
So, How about that I add a new OpenCL kernel file in modules/video/src/opencl/ directory? When the host code check winSize within 16*16 to 24*24, the new kernel will be called. Otherwise, the old kernel - pyrlk.cl - will be used. Is that OK?
There was a problem hiding this comment.
I mean there is no reason to call some implementation (for example, OpenCL kernel) with unsupported parameters.
You state this:
In this kernel, the input winSize bigger than 24x24 will be referred as 24x24, the input winSize smaller than 16x16 will be referred as 16x16.
if this is true, then appropriate parameters check should be added.
All OpenCV tests should not rely on concrete implementation.
So this strange condition should be eliminated, because its justification is related to concrete OpenCL implementation and it will not be accepted.
|
@LukeZheZhu, I think, @alalek means that the check for window size is better be placed to checkParam() method (line 843) that checks whether algorithm parameters are appropriate for OpenCL implementation. BTW, the lower limit (16 x 16) for the window size is quite constraining. People often use window size like 11x11 or 15x15. So it would be nice if someone fixed the kernel to support smaller window size |
dac7226 to
37d324c
Compare
…ector of err. Improve err's precison of the calcOpticalFlowPyrLK OpenCL function and add the relative test.
37d324c to
65be9e1
Compare
|
Hi @vpisarev @alalek , |
|
thank you! 👍 |
resolves #8608
I set the max error of the output err between cpu and gpu as 0.01, which means the test points with std::abs(cpuErr[i] - err[i]) < 0.01 are correct. As the different implementation details between the cpu and gpu code, only 90% test points can pass the test.
Furthermore, the gpu code only support window size within
16*16to24*24. Therefore this patch cannot work correctness when the window size out of this range.In summary, with max differnece of output err between cpu and gpu as 0.01 and window size in the range of
16*16and24*24, this patch can provide 90% correctness.This pullrequest changes
Modified the modules/video/src/opencl/pyrlk.cl to fix the output err
Add relative test in the modules/video/test/ocl/test_optflowpyrlk.cpp.