Skip to content

DON'T MERGE, DNN: disable winograd as default#24587

Closed
zihaomu wants to merge 1 commit intoopencv:4.xfrom
zihaomu:disable_wino_as_default
Closed

DON'T MERGE, DNN: disable winograd as default#24587
zihaomu wants to merge 1 commit intoopencv:4.xfrom
zihaomu:disable_wino_as_default

Conversation

@zihaomu
Copy link
Copy Markdown
Member

@zihaomu zihaomu commented Nov 24, 2023

This PR tries to disable the Winograd compute branch as default, which will greatly affect dnn running performance.

Test at Apple M2 chip, multi-thread:
m2_perf_test.zip

After turn off the Wingrad, 3x3s1 convolution performance drops a lot (4.x vs patch):
image

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

util::getParameter(params, "dilation", "dilation", dilations, true, std::vector<size_t>(kernel.size(), 1));
util::getParameter(params, "adj", "adj", adjust_pads, true, std::vector<size_t>(kernel.size(), 0));
useWinograd = params.get<bool>("use_winograd", true);
useWinograd = params.get<bool>("use_winograd", useWinograd);
Copy link
Copy Markdown
Member Author

@zihaomu zihaomu Nov 24, 2023

Choose a reason for hiding this comment

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

Hi @asmorkalov @vpisarev. Previously, due to this line of code, we actually turned on Winograd by default.

@zihaomu zihaomu changed the title DNN: disable wino as default DNN: disable winograd as default Nov 24, 2023
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

👍

@zihaomu zihaomu changed the title DNN: disable winograd as default DON'T MERGE, DNN: disable winograd as default Nov 24, 2023
@asmorkalov asmorkalov closed this Dec 19, 2023
asmorkalov pushed a commit that referenced this pull request Dec 22, 2023
Try to enable Winograd by default in FP32 mode and disable it by default in FP16 mode #24709

Hopefully, it will resolve regressions since 4.8.1 (see also #24587)

### 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
thewoz pushed a commit to thewoz/opencv that referenced this pull request May 29, 2024
Try to enable Winograd by default in FP32 mode and disable it by default in FP16 mode opencv#24709

Hopefully, it will resolve regressions since 4.8.1 (see also opencv#24587)

### 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants