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.

Sign up for GitHub

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

Jump to bottom

DNN: fused depthwise and add #23096

Merged
merged 1 commit into from Jan 12, 2023
Merged

DNN: fused depthwise and add #23096

merged 1 commit into from Jan 12, 2023

Conversation

zihaomu
Copy link
Member

@zihaomu zihaomu commented Jan 4, 2023 •
edited by alalek

Merge with test data: opencv/opencv_extra#1034
Fixes: #23074

In the previous optimization, we fused the Conv and Add layers. This PR further provides support for Depth-wise Conv and Add layers fusion.

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

@zihaomu zihaomu linked an issue Jan 4, 2023 that may be closed by this pull request
Assertion failed in dnn::runFastConv #23074
Closed
4 tasks
@zihaomu zihaomu mentioned this pull request Jan 4, 2023
Merged
@zihaomu zihaomu force-pushed the issue_23074 branch 3 times, most recently from 161dd13 to ab37a1c Compare January 6, 2023 02:01
@asmorkalov asmorkalov requested a review from rogday January 9, 2023 07:51
@asmorkalov
Copy link
Contributor

@rogday Please take a look.

@asmorkalov asmorkalov added this to the 4.8.0 milestone Jan 9, 2023
alalek
alalek approved these changes Jan 10, 2023
View reviewed changes
Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

LGTM 👍

rogday
rogday reviewed Jan 10, 2023
View reviewed changes
modules/dnn/src/layers/fast_convolution/depthwise_convolution.cpp Outdated
Comment on lines 322 to 326
if (fusedAdd)
out += outptr[out_j];
if (relu)
out = out > 0.f ? out : out*relu_coeff;
outptr[out_j] = out;
outptr[out_j] += out;
Copy link
Member

Choose a reason for hiding this comment

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

Seems suspicious - we add outptr 2 times, is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thx for reviewing. Fixed.

rogday
rogday approved these changes Jan 11, 2023
View reviewed changes
Copy link
Member

@rogday rogday left a comment

Choose a reason for hiding this comment

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

👍

@opencv-pushbot opencv-pushbot merged commit 3d5e3a9 into opencv:4.x Jan 12, 2023
@alalek alalek mentioned this pull request Jan 18, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vpisarev vpisarev vpisarev approved these changes

@alalek alalek alalek approved these changes

@rogday rogday rogday approved these changes

Assignees

@rogday rogday

Labels
bug category: dnn
Projects
None yet
Milestone
4.8.0
Development

Successfully merging this pull request may close these issues.

Assertion failed in dnn::runFastConv
6 participants
@zihaomu @asmorkalov @vpisarev @alalek @rogday @opencv-pushbot