CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
GAPI: Add compare function to perf tests #21150
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
{ | ||
EXPECT_TRUE(cmpF(out_mat_ocv, out_mat_gapi)); | ||
EXPECT_EQ(0, cv::norm(out_mat_ocv, out_mat_gapi, NORM_INF)); | ||
EXPECT_EQ(out_mat_gapi.size(), sz_in); |
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 leave only one EXPECT_TRUE(cmpF(out_mat_gapi, out_mat_ocv));
and remove the rest.
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.
Applied, thanks
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.
Great work!!
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 resolve conflicts as well as address some small comments remaining
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.
LGTM! Thanks!
5209fcc
to
a8fe8f1
Compare
@alexgiving could you please resolve merge conflicts and revert all changes doesn't linked with topic of this PR? |
25e98fa
to
e2ff2d3
Compare
cv::Size sz; | ||
MatType type = -1; | ||
cv::GCompileArgs compile_args; | ||
std::tie(cmpF, opType, sz, type, compile_args) = GetParam();; |
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 remove redundant ;
at the end of line.
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.
Applied
e2ff2d3
to
4d87b07
Compare
Combine(Values(AbsToleranceScalar(0.0).to_compare_f()), | ||
Values(szSmall128, szVGA, sz720p, sz1080p), | ||
Values(CV_8UC1, CV_16UC1, CV_16SC1, CV_32FC1), | ||
Values(cv::compile_args(CORE_CPU)))); | ||
|
||
INSTANTIATE_TEST_CASE_P(AddWeightedPerfTestCPU, AddWeightedPerfTest, | ||
Combine(Values(Tolerance_FloatRel_IntAbs(1e-6, 1).to_compare_f()), |
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.
Could you please try to use AbsExact()
function here?
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.
In that test there is a comparison between (cv::Scalar, cv::Scalar)
, AbsExact()
compare two cv::Mat
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.
Did you means AddWeightedPerfTestCPU
? Are you sure?
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.
I means AddWeightedPerfTestCPU
test case. It compares 2 cv::Mat
. I'm sure. So, please try AbsExact()
.
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.
Applied
Combine(Values(AbsExact().to_compare_f()), | ||
Values(szSmall128, szVGA, sz720p, sz1080p), | ||
Values(CV_8UC1, CV_8UC3, CV_16UC1, CV_16SC1, CV_32FC1), | ||
Values(cv::compile_args(CORE_GPU)))); | ||
|
||
|
||
INSTANTIATE_TEST_CASE_P(DISABLED_ConcatVertVecPerfTestGPU, ConcatVertVecPerfTest, |
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.
Could you please left a comment why this test still disabled?
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.
That test is working now. Enabled
339917d
to
faade90
Compare
740bb86
to
1b38586
Compare
class TransposePerfTest : public TestPerfParams<tuple<compare_f, cv::Size, MatType, cv::GCompileArgs>> {}; | ||
class ResizePerfTest : public TestPerfParams<tuple<compare_f, MatType, int, cv::Size, cv::Size, cv::GCompileArgs>> {}; | ||
class BottleneckKernelsConstInputPerfTest : public TestPerfParams<tuple<compare_f, std::string, cv::GCompileArgs>> {}; | ||
class BottleneckKernelsConstInputPerfTest : public TestPerfParams<tuple<compare_f, std::string, double, double, cv::GCompileArgs>> {}; |
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 remove possibility of configuration scale
parameter. This contradicts the meaning of the test. For this test case fx
and fy
must be strictly equal to 0.5. Always.
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.
Applied
Values("cv/optflow/frames/1080p_00.png", "cv/optflow/frames/720p_00.png", | ||
"cv/optflow/frames/VGA_00.png", "cv/dnn_face/recognition/Aaron_Tippin_0001.jpg"), | ||
Values(0.5), | ||
Values(0.5), |
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 remove this configuration parameters. fx and fy must be equals to 0.5 constantly. It should not be possible to change them.
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.
Applied
5161496
to
4c02e28
Compare
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.
Looks good! Thanks!
AddWeightedPerfTest is not work on OpenCL on GPU (throw termination reason: reached maximum number of iterations/AbsExact error: G-API output and reference output matrixes are not bitexact equal.) with parameters (Any, 5) and (32FC1, Any) [3034] |
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.
Thanks, approved 👍
The only question: How much time our perf test takes now? I believe we should cut the number of tested combinations to fit it into the reasonable amount (as @alalek has indicated a couple of times)
GAPI: Add compare function to perf tests * Add PhasePerfTest and SqrtPerfTest * rebasing * debug tests * remove spaces * Disable DivRCPerfTestGPU * rebase * Applied comments * Correction * Revert parameter changes
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.