Skip to content
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

Test code - GpuMatND & Mat interoperability #2805

Merged
merged 1 commit into from
Feb 9, 2021

Conversation

nglee
Copy link
Contributor

@nglee nglee commented Jan 5, 2021

Main PR: opencv/opencv#19259

opencv=
force_builders=Custom
buildworker:Custom=linux-4
build_image:Custom=ubuntu-cuda:18.04

@nglee nglee force-pushed the dev_gpumatnd1 branch 5 times, most recently from c189b7d to 842d756 Compare January 8, 2021 04:17
@alalek
Copy link
Member

alalek commented Jan 9, 2021

BTW, Please avoid using of # in commit messages (GitHub tries to create wrong cross-links in this case)

@nglee nglee changed the title Test code for opencv #19259 Test code - GpuMatND & Mat interoperability Jan 9, 2021
@nglee nglee force-pushed the dev_gpumatnd1 branch 2 times, most recently from 74127fb to 60319b3 Compare January 12, 2021 03:18
Comment on lines 41 to 48
{
// simple upload, download test for GpuMatND

gmat.upload(gold);
gmat.download(dst);

EXPECT_TRUE(std::equal(gold.begin(), gold.end(), dst.begin()));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'll be great if you split the blocks like this on dedicated parameterized tests with GTest. It simplifies issue triaging and reduces side effects if one block affects another during execution. The test you used as base was implemented in prehistoric period and we try to get rid of large 'doTest' things that does everything.

Copy link
Contributor Author

@nglee nglee Feb 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll split the blocks in the next commit.

Currently, it uses type-parameterized tests. Should I change it to a value parameterized form?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have fixed it.

@@ -0,0 +1,247 @@
#include "test_precomp.hpp"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use short license header: https://github.com/opencv/opencv/wiki/Coding_Style_Guide#file-structure

// This file is part of OpenCV project.
// It is subject to the license terms in the LICENSE file found in the top-level directory
// of this distribution and at http://opencv.org/license.html.


using namespace cv;
using namespace cv::cuda;
using namespace cvtest;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use namespace opencv_test { namespace { like others tests do.

Comment on lines 22 to 24
for (ElemType& elem : ret)
for (int i = 0; i < cn; ++i)
elem[i] = static_cast<CnType>(std::rand());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cv::randu() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have fixed test codes but builds failed because of a merge conflict.

@opencv-pushbot opencv-pushbot merged commit 4e85f8c into opencv:master Feb 9, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants