Add support for Conv1D on OpenCV backend by sl-sergei · Pull Request #18783 · opencv/opencv

sl-sergei · 2020-11-11T22:42:14Z

resolves #18205

merge with extra: opencv/opencv_extra#813

opencv_extra=test_conv1d

force_builders=Custom,Custom Win,Custom Mac

Xbuildworker:Custom=linux-3
Xbuild_image:Custom=ubuntu:18.04
XCPU_BASELINE:Custom=AVX512_SKX
Xdisable_ipp:Custom=ON

build_image:Custom=ubuntu-openvino-2021.1.0:20.04
build_image:Custom Win=openvino-2020.2.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=OFF
test_bigdata:Custom=1
test_filter:Custom=*

modules/dnn/src/layers/convolution_layer.cpp

modules/dnn/src/onnx/onnx_importer.cpp

asmorkalov · 2020-11-12T07:14:32Z

modules/dnn/perf/perf_convolution1d.cpp

+    typedef tuple<Conv1DParamID, tuple<Backend, Target> > Conv1DTestParam_t;
+    typedef TestBaseWithParam<Conv1DTestParam_t> Conv1D;
+
+    PERF_TEST_P_(Conv1D, conv1d)


Indentation.

asmorkalov · 2020-11-12T07:15:56Z

modules/dnn/perf/perf_convolution1d.cpp

+    Target targetId = get<1>(get<1>(GetParam()));
+
+    if (targetId != DNN_TARGET_CPU)
+    throw SkipTestException("Only CPU is supported");


Indentation.

Indentation.

Sorry, my formatter broke (again)

asmorkalov · 2020-11-12T07:16:35Z

modules/dnn/perf/perf_convolution1d.cpp

+    lp.set("kernel_size", kernel);
+    lp.set("pad", pad);
+    if (!padMode.empty())
+    lp.set("pad_mode", padMode);


Indentation.

asmorkalov · 2020-11-12T07:16:57Z

modules/dnn/perf/perf_convolution1d.cpp

+{
+    Mat bias(1, outChannels, CV_32F);
+    randu(bias, -1.0f, 1.0f);
+    lp.blobs.push_back(bias);
+}


Indentation.

modules/dnn/perf/perf_convolution1d.cpp

asmorkalov · 2020-11-12T07:29:33Z

modules/dnn/perf/perf_convolution1d.cpp

+{
+    Mat res = net.forward();
+}
+EXPECT_NEAR(flops, declared_flops, declared_flops * 1e-6);


flops is integer value. Check should be exact without epsilon.

asmorkalov · 2020-11-12T07:30:38Z

modules/dnn/perf/perf_convolution1d.cpp

+net.setPreferableBackend(backendId);
+net.setPreferableTarget(targetId);
+
+Mat output = net.forward();


Please add some check for output result. At least output shape.

Actually, this is a warmup iteration, I don't think it is necessary to check the correctness of output shapes inside performance tests (it is already covered in other tests)

modules/dnn/src/layers/convolution_layer.cpp

sl-sergei · 2020-11-12T15:57:46Z

@dkurt I have updated the code, can you resolve your comments/requests?

sl-sergei · 2020-11-12T16:29:09Z

@asmorkalov Can you resolve your comments/requests?

sl-sergei · 2020-11-12T17:34:33Z

@alalek @dkurt My current changes do not affect test case Layer_Test_Slice.slice_channels_and_batch_17762/3 and some other tests in opencv_test_dnn, but still they are sporadically fail on MYRIAD device. Do you know something about this problem?

alalek · 2020-11-12T17:37:57Z

Ignore these test failures because they are not related to this patch.
These tests fail on other PRs too.

modules/dnn/src/layers/convolution_layer.cpp

dkurt

👍 Thanks!

l-bat · 2020-11-13T12:50:29Z

Could you please add test with variable weights (blobs.empty())?

sl-sergei · 2020-11-13T14:51:58Z

@dkurt @alalek What do you think about OpenCL support? I discussed it today with Vadim Pisarevsky and we consider it as low priority task. @alalek How fast can these changes be merged to 4.* branch? I want to enable it for CUDA backend.

sl-sergei · 2020-11-13T14:52:11Z

Could you please add test with variable weights (blobs.empty())?

Sure

alalek · 2020-11-13T15:09:26Z

"Merge 3.4" is expected on this week (at 21:00 UTC today or tomorrow)

alalek · 2020-11-13T20:01:55Z

You can skip implementation of OpenCL optimizations, but it should be properly handled.

There is general policy about DNN layers implementation:

layer .forward() should not fail with exception due to lack of optimization implementation
layer + DNN engine should fallback on OpenCV+CPU in such cases if optimization doesn't support passed parameters
OpenCV+CPU should be always supported

/cc @vpisarev

alalek · 2020-11-13T22:02:33Z

but still they are sporadically fail on MYRIAD device

You have used outdated versions of IE for testing.
Actual versions which should be used by default are stored here: https://github.com/opencv/opencv/wiki/CI-configuration ("DNN testing" section)

Add support for Conv1D on OpenCV backend

3ce6fd7

sl-sergei requested review from dkurt and l-bat November 11, 2020 22:42

sl-sergei mentioned this pull request Nov 11, 2020

Testdata for Conv1d layer opencv/opencv_extra#813

Merged

disable tests on other targets/backends

adb47af

dkurt reviewed Nov 12, 2020

View reviewed changes

modules/dnn/src/layers/convolution_layer.cpp Show resolved Hide resolved

dkurt reviewed Nov 12, 2020

View reviewed changes

modules/dnn/src/layers/convolution_layer.cpp Outdated Show resolved Hide resolved

dkurt reviewed Nov 12, 2020

View reviewed changes

modules/dnn/src/onnx/onnx_importer.cpp Outdated Show resolved Hide resolved

asmorkalov requested changes Nov 12, 2020

View reviewed changes

Fix formatting

453cf77

sl-sergei force-pushed the fix_conv1d branch from c6ae9c1 to 453cf77 Compare November 12, 2020 07:58

sl-sergei added 3 commits November 12, 2020 11:00

Restore comment

d867b8c

Remove unnecessary flag and fix test logic

46d6e3b

Fix perf test

9e18944

dkurt reviewed Nov 12, 2020

View reviewed changes

modules/dnn/src/layers/convolution_layer.cpp Outdated Show resolved Hide resolved

dkurt reviewed Nov 12, 2020

View reviewed changes

modules/dnn/src/layers/convolution_layer.cpp Outdated Show resolved Hide resolved

fix braces

b9a742b

sl-sergei removed the request for review from l-bat November 12, 2020 13:51

dkurt reviewed Nov 12, 2020

View reviewed changes

modules/dnn/src/layers/convolution_layer.cpp Outdated Show resolved Hide resolved

dkurt reviewed Nov 12, 2020

View reviewed changes

modules/dnn/src/layers/convolution_layer.cpp Outdated Show resolved Hide resolved

dkurt reviewed Nov 12, 2020

View reviewed changes

modules/dnn/src/layers/convolution_layer.cpp Outdated Show resolved Hide resolved

dkurt reviewed Nov 12, 2020

View reviewed changes

modules/dnn/src/layers/convolution_layer.cpp Outdated Show resolved Hide resolved

Fix indentation, assert check and remove unnecessary condition

1e330a1

sl-sergei requested review from asmorkalov and dkurt November 12, 2020 17:47

dkurt reviewed Nov 13, 2020

View reviewed changes

modules/dnn/src/layers/convolution_layer.cpp Outdated Show resolved Hide resolved

dkurt reviewed Nov 13, 2020

View reviewed changes

modules/dnn/src/layers/convolution_layer.cpp Outdated Show resolved Hide resolved

dkurt reviewed Nov 13, 2020

View reviewed changes

modules/dnn/src/layers/convolution_layer.cpp Show resolved Hide resolved

Remove unnecessary changes

a409178

dkurt approved these changes Nov 13, 2020

View reviewed changes

alalek assigned dkurt Nov 13, 2020

Add test cases for variable weights and bias

5d36ebf

alalek added 2 commits November 13, 2020 21:06

dnn(conv): fallback on OpenCV+CPU instead of fail

6558083

coding style

fa1a451

alalek merged commit 61144f9 into opencv:3.4 Nov 13, 2020

alalek mentioned this pull request Nov 13, 2020

Merge 3.4 #18800

Merged

alalek mentioned this pull request Nov 27, 2020

Merge 4.x #18951

Merged

This was referenced Dec 8, 2020

Preparations for enabling 1D support for Conv and Pool operations on CUDA #19057

Merged

Conv1D and Pool1D for CUDA backend #19058

Merged

Uh oh!

Conversation

sl-sergei commented Nov 11, 2020 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sl-sergei commented Nov 12, 2020

Uh oh!

sl-sergei commented Nov 12, 2020

Uh oh!

sl-sergei commented Nov 12, 2020

Uh oh!

alalek commented Nov 12, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dkurt left a comment • 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!

l-bat commented Nov 13, 2020

Uh oh!

sl-sergei commented Nov 13, 2020

Uh oh!

sl-sergei commented Nov 13, 2020

Uh oh!

alalek commented Nov 13, 2020

Uh oh!

alalek commented Nov 13, 2020

Uh oh!

alalek commented Nov 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sl-sergei commented Nov 11, 2020 •

edited by alalek

Loading

dkurt left a comment •

edited

Loading