dnn: add exhaustive fusion tests, enable more eltwise fusions by YashasSamaga · Pull Request #17976 · opencv/opencv

YashasSamaga · 2020-07-29T11:23:47Z

fixes #17946 for 3.4

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 OpenCV (BSD) License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
The PR is proposed to proper branch
There is reference to 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

force_builders=Custom,Custom Win,Custom Mac
build_image:Custom=ubuntu-openvino-2020.2.0:16.04
build_image:Custom Win=openvino-2020.3.0
build_image:Custom Mac=openvino-2020.2.0

test_modules:Custom=dnn,python2,python3,java
test_modules:Custom Win=dnn,python2,python3,java
test_modules:Custom Mac=dnn,python2,python3,java

buildworker:Custom=linux-1
# disabled due high memory usage: test_opencl:Custom=ON
test_opencl:Custom=ON
test_bigdata:Custom=1
test_filter:Custom=*TestLayerFusion*

test_opencl:Custom Win=ON
test_bigdata:Custom Win=1
test_filter:Custom Win=*TestLayerFusion*

YashasSamaga · 2020-07-29T11:26:56Z

modules/dnn/test/test_layers.cpp

+    static testing::internal::ParamGenerator<std::string> activationLayersList()
+    {
+        // TODO: automate list generation
+        return Values("ReLU", "ReLU6", "ChannelsPReLU", "TanH", "Swish", "Mish", "Sigmoid", "ELU", "AbsVal", "BNLL", "Power");


I was wondering if it would be possible to automatically generate this list.

YashasSamaga · 2020-07-29T11:29:10Z

modules/dnn/test/test_layers.cpp

+
+    static testing::internal::ParamGenerator<tuple<Backend, Target>> dnnBackendsAndTargetsForFusionTests()
+    {
+        return dnnBackendsAndTargets(false, false, false, false); // only OpenCL


The fusion tests are checked for OpenCL only in 3.4. I am not sure if these should be enabled for other backends since they don't fuse activations or eltwise.

Note that this line might require special handling in 3.4 merge PR. The withCUDA appears before withNgraph on master. The tests should be turned on for CUDA.

YashasSamaga · 2020-07-29T11:35:46Z

Fusion information for the tests added by this PR after enabling debug printfs: https://gist.github.com/YashasSamaga/70e7fe1fc8c9d670c73eb7ea47ec592d

The tests do not ensure that fusions do take place; they just check that the outputs match with the outputs from OCV CPU backend with fusions disabled. If a future PR accidentally stops a fusion, the tests will still pass since the unfused outputs will be corrected. I don't know if there is a way to enforce that fusions do take place.

asmorkalov · 2020-07-30T12:32:51Z

@dkurt @sl-sergei Could you look at it?

YashasSamaga · 2020-07-30T12:35:27Z

modules/dnn/src/dnn.cpp

                                        nextActivLayer = nextData->layerInstance.dynamicCast<ActivationLayer>();

-                                    if( !nextActivLayer.empty() && pinsToKeep.count(lpNext) == 0 &&
+                                    if( !nextActivLayer.empty() &&


This change need not be propagated to master. The eltwise fusion logic is divergent. #17939 applies the same fix on master.

modules/dnn/test/test_layers.cpp

YashasSamaga · 2020-08-01T10:41:22Z

Can I squash the commits and rebase? Asking because it could break any reviews in progress.

YashasSamaga · 2020-08-04T07:06:40Z

Tests seem to be failing in CI. I am not able to reproduce them locally.

C++ exception with description "OpenCV(3.4.11-dev) /build/precommit_custom_linux/3.4/opencv/modules/dnn/test/test_layers.cpp:2128: error: (-215:Assertion failed) refTimings[i] != 0.0 in function 'test'

This indicates that OCV CPU backend with fusion disabled had a layer timing of 0.0. I assumed that a zero timing implies fusion.

And *TestLayerFusion* can be used as the gtest filter to run all the tests added by this PR. *ConvolutionActivation* won't run all of them.

alalek · 2020-08-04T09:46:04Z

@YashasSamaga We will take a look on these failures.

modules/dnn/test/test_layers.cpp

alalek · 2020-08-07T15:23:43Z

@YashasSamaga Please fix copy-parse typo (should fix current red custom builds too) and squash commits onto one.

YashasSamaga · 2020-08-07T17:46:06Z

modules/dnn/test/test_layers.cpp

+        float conv_init_magnitude = 1.0f / in_channels / kernel_h / kernel_w;
+        int weightsShape[] = {num_filters, in_channels, kernel_h, kernel_w};
+        Mat weights(4, &weightsShape[0], CV_32F);
+        randu(weights, -conv_init_magnitude, conv_init_magnitude);


I am not sure if this is a good initialization. The weights might be too small.

Why do you think if that's a problem? I think it also good chance to track if we have no regressions with such kind of optimizations: #17295

It will squeeze all weights to a very small range [-0.007, 0.007]. I don't think there is any serious problem but not sure.

modules/dnn/test/test_layers.cpp

YashasSamaga · 2020-08-07T18:05:46Z

@YashasSamaga Please fix copy-parse typo (should fix current red custom builds too) and squash commits onto one.

Done. Sorry for the typo.

Tests will fail on master if these tests are enabled for CUDA backend. Need to add some code for each test case indicating which layers are fused (the code enforcing the fusions expects no layers to be fused by default). I'll make a PR after this arrives in master to enable these tests for the CUDA backend.

modules/dnn/test/test_layers.cpp

dkurt

👍 Thank you!

YashasSamaga commented Jul 29, 2020

View reviewed changes

YashasSamaga marked this pull request as ready for review July 29, 2020 12:31

asmorkalov requested review from dkurt and sl-sergei July 30, 2020 12:32

YashasSamaga commented Jul 30, 2020

View reviewed changes

dkurt reviewed Jul 30, 2020

View reviewed changes

modules/dnn/test/test_layers.cpp Show resolved Hide resolved

alalek reviewed Aug 6, 2020

View reviewed changes

modules/dnn/test/test_layers.cpp Outdated Show resolved Hide resolved

add eltwise fusion tests, enable more eltwise fusions

52f7bf4

YashasSamaga force-pushed the dnn-fusion-tests-fix-ocl branch from c9c8324 to 52f7bf4 Compare August 7, 2020 17:33

YashasSamaga commented Aug 7, 2020

View reviewed changes

dkurt reviewed Aug 7, 2020

View reviewed changes

modules/dnn/test/test_layers.cpp Show resolved Hide resolved

merge weighted eltwise tests with eltwise tests

484114c

dkurt reviewed Aug 11, 2020

View reviewed changes

modules/dnn/test/test_layers.cpp Show resolved Hide resolved

dkurt approved these changes Aug 13, 2020

View reviewed changes

alalek assigned dkurt Aug 13, 2020

alalek merged commit 2171cae into opencv:3.4 Aug 13, 2020

alalek mentioned this pull request Aug 14, 2020

Merge 3.4 #18099

Merged

Uh oh!

Conversation

YashasSamaga commented Jul 29, 2020 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

YashasSamaga Jul 29, 2020

Choose a reason for hiding this comment

Uh oh!

YashasSamaga Jul 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YashasSamaga commented Jul 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asmorkalov commented Jul 30, 2020

Uh oh!

YashasSamaga Jul 30, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

YashasSamaga commented Aug 1, 2020

Uh oh!

YashasSamaga commented Aug 4, 2020

Uh oh!

alalek commented Aug 4, 2020

Uh oh!

Uh oh!

alalek commented Aug 7, 2020

Uh oh!

YashasSamaga Aug 7, 2020

Choose a reason for hiding this comment

Uh oh!

dkurt Aug 10, 2020

Choose a reason for hiding this comment

Uh oh!

YashasSamaga Aug 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

YashasSamaga commented Aug 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dkurt left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

YashasSamaga commented Jul 29, 2020 •

edited by alalek

Loading

YashasSamaga Jul 29, 2020 •

edited

Loading

YashasSamaga commented Jul 29, 2020 •

edited

Loading

YashasSamaga Aug 10, 2020 •

edited

Loading

YashasSamaga commented Aug 7, 2020 •

edited

Loading