Support Convolution3D layer on IE backend by l-bat · Pull Request #14301 · opencv/opencv

l-bat · 2019-04-11T12:06:27Z

Merge with extra: opencv/opencv_extra#594

force_builders=Custom,Linux AVX2
buildworker:Custom=linux-2
docker_image:Custom=ubuntu-openvino-2019r1:16.04
test_opencl:Custom=ON
test_bigdata:Custom=1
test_filter:Custom=*
test_modules:Custom=dnn

ci_branch:Linux AVX2=no-checks
docker_image:Linux AVX2=ubuntu-openvino-2018r5:16.04
test_opencl:Linux AVX2=OFF
buildworker:Linux AVX2=linux-1
build_parallel_tests:Linux AVX2=1
test_bigdata:Linux AVX2=1
test_module:Linux AVX2=dnn,python2,python3,java
tests_filter:Linux AVX2=*

buildworker:Mac OpenCL=macosx-1
build_image:Mac OpenCL=openvino-2019r1
test_module:Mac OpenCL=dnn,python2,python3,java

dkurt · 2019-04-17T11:31:06Z

modules/dnn/test/test_onnx_importer.cpp

+TEST_P(Test_ONNX_layers, Convolution3D)
+{
+    if (backend == DNN_BACKEND_OPENCV || target != DNN_TARGET_CPU)
+        throw SkipTestException("");


Let's replace to

if (backend != DNN_BACKEND_INFERENCE_ENGINE || target != DNN_TARGET_CPU) throw SkipTestException("Only DLIE backend on CPU is supported");

dkurt · 2019-04-17T11:38:40Z

modules/dnn/src/tensorflow/tf_importer.cpp

+                int kernel[] = {layerParams.blobs[0].size[2], layerParams.blobs[0].size[3], layerParams.blobs[0].size[4]};
+                layerParams.set("kernel",  DictValue::arrayInt(kernel, 3));
+                setDilations(layerParams, layer);
+            }


Can we generalize it? In example,

Mat weights = layerParams.blobs[0]; layerParams.set("kernel", DictValue::arrayInt(&weights.size[2], weights.dims - 2));

dkurt · 2019-04-17T11:39:32Z

modules/dnn/src/tensorflow/tf_importer.cpp

+            else if (layerParams.blobs[0].dims == 5) {
+                int kernel[] = {layerParams.blobs[0].size[2], layerParams.blobs[0].size[3], layerParams.blobs[0].size[4]};
+                layerParams.set("kernel",  DictValue::arrayInt(kernel, 3));
+                setDilations(layerParams, layer);


Why we need dilations in case of 5D kernel but not for 4D one?

dkurt · 2019-04-17T11:44:30Z

modules/dnn/src/layers/convolution_layer.cpp

+#if INF_ENGINE_VER_MAJOR_GE(INF_ENGINE_RELEASE_2018R5)
+        InferenceEngine::Builder::ConvolutionLayer ieLayer(name);
+
+        ieLayer.setKernel({(size_t)kernel.get<int>(0), (size_t)kernel.get<int>(1), (size_t)kernel.get<int>(2)});


Please experiment with merging Convolution3D into the Convolution layer. IE backend receives vectors as convolution's hyperparameters so it might looks like:

// std::vector<size_t> kernel: height+width for Convolution and depth+height+width for Convolution3D ieLayer.setKernel(kernel);

dkurt · 2019-04-19T14:39:59Z

modules/dnn/src/layers/convolution_layer.cpp

+        ieLayer.setStrides(std::vector<size_t>(strides.begin(), strides.end()));
+        ieLayer.setDilation(std::vector<size_t>(dilations.begin(), dilations.end()));
+        ieLayer.setPaddingsBegin(std::vector<size_t>(pads.begin(), pads.begin() + pads.size() / 2));
+        ieLayer.setPaddingsEnd(std::vector<size_t>(pads.begin() + pads.size() / 2, pads.end()));


Can we just use std::vector<size_t> data type for all the kernel_size, pads, strides and dilations?

dkurt · 2019-04-19T14:41:50Z

modules/dnn/src/layers/convolution_layer.cpp

+        CV_Assert(input->dims.size() == 4 || input->dims.size() == 5);

-        const int inpCn = input->dims[2];  // NOTE: input->dims are reversed (whcn)
+        const int inpCn = input->dims[input->dims.size() - 2];  // NOTE: input->dims are reversed (whcn or whdcn)


Please rename whcn to WHIO (and whdcn to WHDIO correspondingly) due there is no batch (n) and channels (c) in the kernel but there are input channels (I) and output channels (O).

dkurt · 2019-04-19T14:45:54Z

modules/dnn/src/layers/convolution_layer.cpp

        const Mat &input = inputs[0];
-        CV_Assert(input.dims == 4 && (input.type() == CV_32F || input.type() == CV_64F || input.type() == CV_16S));
-        for (size_t i = 0; i < inputs.size(); i++)
+        CV_Assert((input.dims == 4 || input.dims == 5) && (input.type() == CV_32F || input.type() == CV_64F || input.type() == CV_16S));


I think that we can remove input.type() == CV_64F check due there is no double precision support.

dkurt · 2019-04-19T14:46:22Z

modules/dnn/src/layers/convolution_layer.cpp

-        CV_Assert(input.dims == 4 && (input.type() == CV_32F || input.type() == CV_64F || input.type() == CV_16S));
-        for (size_t i = 0; i < inputs.size(); i++)
+        CV_Assert((input.dims == 4 || input.dims == 5) && (input.type() == CV_32F || input.type() == CV_64F || input.type() == CV_16S));
+        for (int i = 0; i < inputs.size(); i++)


why not size_t?

dkurt · 2019-04-19T14:54:25Z

modules/dnn/src/layers/convolution_layer.cpp

-            out.height = (inpH + 2 * pad.height - (dilation.height * (kernel.height - 1) + 1)) / stride.height + 1;
-            out.width = (inpW + 2 * pad.width - (dilation.width * (kernel.width - 1) + 1)) / stride.width + 1;
+            for (int i = 0; i < inpShape.size(); i++)
+                outShape.push_back((inpShape[i] + pads[i] + pads[i + pads.size() / 2] - dilations[i] * (kernel_size[i] - 1) - 1) / strides[i] + 1);


I think it also make sense for us to split paddings vector to begins and ends as IE expects. Can you try?

dkurt · 2019-04-23T06:12:02Z

modules/dnn/src/layers/convolution_layer.cpp

-                  adjustPad.height < stride.height);
+            adjust_pads.resize(2);
+            adjust_pads[0] = params.get<int>("adj_h", 0);
+            adjust_pads[1] = params.get<int>("adj_w", 0);


I guess that we can have adjust paddings with 3D convolution as well.

Now Deconvolution layer is supported only for 4D input

modules/dnn/src/layers/convolution_layer.cpp

dkurt · 2019-04-23T06:23:15Z

modules/dnn/src/layers/convolution_layer.cpp

-        (dilation.height == 1 && dilation.width == 1);
+        return (kernel_size.back() == 1 && kernel_size[kernel_size.size() - 2] == 1) &&
+               (strides.back() == 1 && strides[strides.size() - 2] == 1) &&
+               (dilations.back() == 1 && dilations[dilations.size() - 2] == 1);


Due is1x1 method is called only int the default implementation for 2D convs only we can keep cv::Size checks because they will be actual.

modules/dnn/src/layers/convolution_layer.cpp

dkurt · 2019-04-23T06:31:22Z

modules/dnn/src/layers/convolution_layer.cpp

        {
-            if (INF_ENGINE_RELEASE >= 2018050000 && (adjustPad.height || adjustPad.width))
+            if (kernel_size.size() == 3)
+                return (INF_ENGINE_RELEASE >= 2018050000 && preferableTarget == DNN_TARGET_CPU);


3D deconvolution?

modules/dnn/src/layers/layers_common.cpp

dkurt · 2019-04-23T07:15:28Z

modules/dnn/src/layers/layers_common.cpp

+        if (pads_begin.size() == kernel.size() - 1 && pads_end.size() == kernel.size() - 1) {
+            pads_begin.push_back(pads_begin.back());
+            pads_end.push_back(pads_end.back());
+        }


I think that we need to remove

if (parameter.size() == 1) { parameter.push_back(parameter.back()); }

above and before the final checks do something like

if (pads_begin.size() == 1) { pads_begin.resize(kernel.size(), pads_begin[0]); }

modules/dnn/src/tensorflow/tf_importer.cpp

dkurt · 2019-04-25T07:22:15Z

modules/dnn/src/layers/convolution_layer.cpp

+            if (kernel_size.size() == 3)
+                return false;
+
+            if (INF_ENGINE_RELEASE >= 2018050000 && (adjust_pads[0] || adjust_pads[1]))


We can still check adjustPad.height and adjustPad.width and dilation.width with dilation.height here (2D convolution).

modules/dnn/test/test_tf_importer.cpp

modules/dnn/src/layers/convolution_layer.cpp

dkurt · 2019-04-26T12:24:13Z

modules/dnn/src/layers/convolution_layer.cpp

+        dims.push_back(inputs[0][0]);
+        dims.push_back(outCn);
+        dims.insert(dims.end(), outShape.begin(), outShape.end());
+        outputs.resize(inputs.size(), shape(&dims[0], dims.size()));


Let's push inputs[0][0] and outCn right into outShape at the beginning.

dkurt · 2019-04-26T12:24:59Z

modules/dnn/src/layers/convolution_layer.cpp

+        std::vector<int> inpShape;
+        for (int i = 2; i < inputs[0].size(); i++) {
+            inpShape.push_back(inputs[0][i]);
+        }


std::vector<int> inpShape(inputs[0].begin() + 2, inputs[0].end());

dkurt · 2019-04-26T12:30:29Z

modules/dnn/src/layers/pooling_layer.cpp

-            return backendId == DNN_BACKEND_OPENCV ||
-                   (backendId == DNN_BACKEND_HALIDE && haveHalide() &&
-                   (type == MAX || (type == AVE && !pad_t && !pad_l && !pad_b && !pad_r)));
+        return (kernel_size.size() != 3) && (backendId == DNN_BACKEND_OPENCV ||


kernel_size.size() == 2

to be more certain

In Layer_Test_ROIPooling.Accuracy "kernel_size.size() = 0

dkurt · 2019-04-26T13:00:31Z

modules/dnn/src/layers/layers_common.cpp

    }
+    util::checkSize(size, pads_begin);
+    util::checkSize(size, pads_end);
+    util::checkSize(size, strides);


I think we can align it to kernel size here because getKernelSize already increases it up to 2.

dkurt · 2019-04-26T13:08:20Z

modules/dnn/src/layers/layers_common.cpp

+    CV_Assert_N(kernel.size() == pads_begin.size() || pads_begin.size() == 1, pads_begin.size() == pads_end.size());
+    pads_begin.resize(kernel.size(), pads_begin[0]);
+    pads_end.resize(kernel.size(), pads_end[0]);
+    CV_Assert(pads_begin.size() == pads_end.size() && pads_begin.size() == kernel.size());


It seems to me that at this point kernel.size() must be equal to pads_begin.size() and pads_end.size()`. May you check this?

dkurt · 2019-04-29T05:55:44Z

modules/dnn/src/layers/convolution_layer.cpp

        if (backendId == DNN_BACKEND_INFERENCE_ENGINE)
        {
+            if (kernel_size.size() == 3)
+                return false;


I think that it's better to have a NotImplemented assertion for kernel_size.size() != 2 at the beginning of supportBackend method.

dkurt · 2019-04-29T06:01:51Z

modules/dnn/src/layers/convolution_layer.cpp

-
-        pad.width = pad_l;
-        pad.height = pad_t;
+         std::vector<int> inpShape;


Extra offsets?

modules/dnn/src/layers/layers_common.cpp

dkurt · 2019-04-29T06:09:15Z

modules/dnn/src/layers/layers_common.cpp

+            if (hasDefault)
            {
-                parameterH = parameterW = defaultValue;
+                parameter.push_back(defaultValue);


The same as above,

parameter.assign(2, defaultValue);

A single kernel_size, pad, stride etc. are observed only with Caffe framework which has no 3d convolutions.

dkurt · 2019-04-29T06:10:26Z

modules/dnn/src/layers/layers_common.cpp


-    CV_Assert(kernelH > 0 && kernelW > 0);
+    if (kernel.size() == 1)
+        kernel.resize(2, kernel[0]);


Considering proposed changes in the getParameter we won't need any other alignments.

dkurt · 2019-04-29T15:29:13Z

modules/dnn/src/layers/pooling_layer.cpp

        {
            type = ROI;
+            pads_begin.resize(1, 0);
+            pads_end.resize(1, 0);


Do we still need this?

dkurt · 2019-04-29T15:29:22Z

modules/dnn/src/layers/pooling_layer.cpp

        {
            type = PSROI;
+            pads_begin.resize(1, 0);
+            pads_end.resize(1, 0);


The same as above

modules/dnn/src/layers/layers_common.cpp

dkurt · 2019-04-30T08:17:55Z

modules/dnn/src/layers/layers_common.cpp

-            else
-            {
-                return false;
-            }


Please keep

else { return false; }

dkurt · 2019-04-30T08:18:21Z

modules/dnn/src/layers/layers_common.cpp

    if (params.has("pad_mode"))
-    {
        padMode = params.get<String>("pad_mode");
-    }


Please keep

if (params.has("pad_mode")) { padMode = params.get<String>("pad_mode"); }

dkurt · 2019-04-30T08:18:46Z

modules/dnn/src/layers/layers_common.cpp

    else
-    {
        CV_Error(Error::StsError, "Unsupported padding mode");
-    }


Please keep

else { CV_Error(Error::StsError, "Unsupported padding mode"); }

dkurt · 2019-04-30T08:19:34Z

modules/dnn/src/layers/pooling_layer.cpp

-                   (type == MAX || (type == AVE && !pad_t && !pad_l && !pad_b && !pad_r)));
+        return (kernel_size.empty() || kernel_size.size() == 2) && (backendId == DNN_BACKEND_OPENCV ||
+               (backendId == DNN_BACKEND_HALIDE && haveHalide() &&
+               (type == MAX || (type == AVE && !pad_t && !pad_l && !pad_b && !pad_r))));


Please keep aligned.

dkurt

Looks good to merge, thanks!

l-bat force-pushed the conv3d branch from aa87cf7 to ccb73c4 Compare April 15, 2019 05:57

dkurt reviewed Apr 17, 2019

View reviewed changes

dkurt reviewed Apr 19, 2019

View reviewed changes

l-bat added 7 commits April 22, 2019 16:23

Add Convolution3D layer

942c6b3

Disable CXX11

5be5041

Fixed tests

3a74fed

Add Pooling3D layer

3c5b8b2

Merge Conv2d with Conv3d and Pool2d with Pool3d layers

07a8b7b

Split pads

a386030

Add Deconvolution layer

3b8f490

l-bat force-pushed the conv3d branch from 96f9407 to 3b8f490 Compare April 22, 2019 15:57

dkurt reviewed Apr 23, 2019

View reviewed changes

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

dkurt reviewed Apr 23, 2019

View reviewed changes

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

dkurt reviewed Apr 23, 2019

View reviewed changes

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

dkurt reviewed Apr 23, 2019

View reviewed changes

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

dkurt reviewed Apr 23, 2019

View reviewed changes

modules/dnn/src/tensorflow/tf_importer.cpp Show resolved Hide resolved

Refactoring

5e3605a

l-bat force-pushed the conv3d branch from 11362a0 to 5e3605a Compare April 24, 2019 10:56

dkurt reviewed Apr 25, 2019

View reviewed changes

l-bat force-pushed the conv3d branch from 2c960f1 to 26debd5 Compare April 25, 2019 12:08

Deduplication

26debd5

dkurt reviewed Apr 26, 2019

View reviewed changes

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

dkurt reviewed Apr 26, 2019

View reviewed changes

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

dkurt reviewed Apr 26, 2019

View reviewed changes

Refactoring

70779ef

dkurt reviewed Apr 29, 2019

View reviewed changes

l-bat mentioned this pull request Apr 29, 2019

conv3d layer support #13933

Closed

dkurt reviewed Apr 29, 2019

View reviewed changes

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

l-bat force-pushed the conv3d branch from 7483a9c to 6bd91b8 Compare April 30, 2019 07:24

dkurt reviewed Apr 30, 2019

View reviewed changes

modules/dnn/src/layers/layers_common.cpp

else

{

return false;

}

Copy link
Copy Markdown

Member

dkurt Apr 30, 2019

Choose a reason for hiding this comment

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

Please keep

else { return false; }

dkurt reviewed Apr 30, 2019

View reviewed changes

l-bat force-pushed the conv3d branch 3 times, most recently from e1b13d8 to 749a9fd Compare April 30, 2019 13:04

Add utils for Convolution and Pooling layers

749a9fd

dkurt approved these changes Apr 30, 2019

View reviewed changes

alalek assigned dkurt Apr 30, 2019

alalek merged commit 77fa59c into opencv:3.4 Apr 30, 2019

This was referenced Apr 30, 2019

Merge 3.4 #14462

Merged

DNN: failed 2 TensorFlow tests in Win32 / Linux32 builders (2019-04-30) #14464

Closed

YashasSamaga mentioned this pull request Dec 12, 2020

Conv1D and Pool1D for CUDA backend #19058

Merged

Uh oh!

Conversation

l-bat commented Apr 11, 2019 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

dkurt Apr 19, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dkurt Apr 23, 2019 • 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!

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!

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!

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!

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

l-bat commented Apr 11, 2019 •

edited by alalek

Loading

dkurt Apr 19, 2019 •

edited

Loading

dkurt Apr 23, 2019 •

edited

Loading