Skip to content

DNN/CUDA: Solve the bug of same shape broadcast with CUDA#23560

Merged
asmorkalov merged 1 commit intoopencv:4.xfrom
WanliZhong:eltwise_cuda_bug
May 16, 2023
Merged

DNN/CUDA: Solve the bug of same shape broadcast with CUDA#23560
asmorkalov merged 1 commit intoopencv:4.xfrom
WanliZhong:eltwise_cuda_bug

Conversation

@WanliZhong
Copy link
Copy Markdown
Member

@WanliZhong WanliZhong commented Apr 28, 2023 •
edited
Loading

Merged after #23557

Fix yolo regression error mentioned in e3e1f70#commitcomment-110475963

After fix this bug, the brute force test mentioned in #23556 will all passed with CUDA. (different shape broadcast will fallback to cpu)

Test is added in PR #23557

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@WanliZhong WanliZhong added bug category: gpu/cuda (contrib) OpenCV 4.0+: moved to opencv_contrib category: dnn labels Apr 28, 2023
@WanliZhong WanliZhong added this to the 4.8.0 milestone Apr 28, 2023
@WanliZhong WanliZhong requested review from rogday and zihaomu April 28, 2023 09:35
@WanliZhong
Copy link
Copy Markdown
Member Author

WanliZhong commented Apr 28, 2023 •
edited
Loading

@peters Hi, could you help to test if this pr will run yolov7-tiny correctly mentioned in e3e1f70#commitcomment-110475963? Thanks!

WanliZhong referenced this pull request Apr 28, 2023
@asmorkalov
Merge pull request #23528 from WanliZhong:issue23278
e3e1f70 
DNN/CUDA: make 'abcd op 1b11' broadcast eltwise operator support cuda
@peters
Copy link
Copy Markdown
Contributor

peters commented Apr 28, 2023

@WanliZhong This PR fixes the yolov7-tiny inference problem. All tests are now green on our side! Thank you :)

@WanliZhong
Copy link
Copy Markdown
Member Author

WanliZhong commented Apr 28, 2023

@peters That's good! Thanks!

@zihaomu zihaomu added the pr: needs test New functionality requires minimal tests set label May 5, 2023
zihaomu
zihaomu approved these changes May 11, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@zihaomu zihaomu left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@zihaomu zihaomu added test and removed pr: needs test New functionality requires minimal tests set labels May 11, 2023
@WanliZhong
Copy link
Copy Markdown
Member Author

WanliZhong commented May 11, 2023

Test results after merged PR #23557 and this PR
image

@opencv-alalek
Copy link
Copy Markdown
Contributor

opencv-alalek commented May 12, 2023

Lets re-enable disabled tests. They are fine on CPU target and pass on CUDA target (but they are slow in the current implementation).

On CUDA target they could be skipped (if performance is critical) in this way:

    if (backendId == DNN_BACKEND_CUDA && ...optional conditionals to select skipped tests...)
        applyTestTag(CV_TEST_TAG_DNN_SKIP_CUDA);

or

    if (backendId == DNN_BACKEND_CUDA && ...optional conditionals...)
        applyTestTag(CV_TEST_TAG_VERYLONG);

To manually run "skipped" tests by tags add this option: --test_tag_enable=dnn_skip_cuda / --test_tag_force=dnn_skip_cuda (see #21088 discussions for details and #12783 for implementation)

@WanliZhong
Copy link
Copy Markdown
Member Author

WanliZhong commented May 12, 2023

Thanks @opencv-alalek ! I will enable it.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented May 16, 2023

@opencv-alalek All tests are green. Should I merge with enabled or disabled brute-force tets?

opencv-alalek
opencv-alalek reviewed May 16, 2023
View reviewed changes
modules/dnn/test/test_layers.cpp
int target = get<1>(backend_target);

if (backend == DNN_BACKEND_CUDA && dim > 4)
applyTestTag(CV_TEST_TAG_LONG);
Copy link
Copy Markdown
Contributor

@opencv-alalek opencv-alalek May 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

BTW, Looks like no tests are skipped on CI.

CV_TEST_TAG_VERYLONG tests are skipped by default.
CV_TEST_TAG_LONG tests should be skipped on embedded platforms or coverage/valgrind build configurations (though extra parameter on CI).

Copy link
Copy Markdown
Member Author

@WanliZhong WanliZhong May 16, 2023

Choose a reason for hiding this comment

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

Longer than 5s but less than 20s, so it should use LONG tag

image

Copy link
Copy Markdown
Contributor

@opencv-alalek opencv-alalek May 16, 2023

Choose a reason for hiding this comment

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

Ok, lets run all CUDA tests.

@asmorkalov asmorkalov merged commit 59ca444 into opencv:4.x May 16, 2023
@WanliZhong WanliZhong deleted the eltwise_cuda_bug branch May 16, 2023 12:33
@asmorkalov asmorkalov mentioned this pull request May 31, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@opencv-alalek opencv-alalek opencv-alalek approved these changes

@zihaomu zihaomu zihaomu approved these changes

@rogday rogday Awaiting requested review from rogday

Assignees

No one assigned

Labels

bug category: dnn category: gpu/cuda (contrib) OpenCV 4.0+: moved to opencv_contrib test

Projects

None yet

Milestone

4.8.0

Development

Successfully merging this pull request may close these issues.

5 participants

@WanliZhong @peters @opencv-alalek @asmorkalov @zihaomu