Added Steps support in DNN Slice layer by LupusSanctus · Pull Request #19546 · opencv/opencv

LupusSanctus · 2021-02-16T22:22:55Z

Merge with extra: opencv/opencv_extra#854

Fixes #18920: added steps parameter support for Slice layer

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 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-2021.2.0:20.04
build_image:Custom Win=openvino-2021.1.0
build_image:Custom Mac=openvino-2021.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/slice_layer.cpp

LupusSanctus · 2021-03-03T15:37:42Z

@alalek, the code was corrected in accordance with comments. Is it needed to rebase this patch on 3.4 branch?

alalek · 2021-03-03T21:09:55Z

Is it needed to rebase this patch on 3.4 branch

We will backport it separately later (there is some CUDA-specific code).

asmorkalov · 2021-03-09T11:40:45Z

/cc @asenyaev

LupusSanctus · 2021-03-09T20:51:02Z

@alalek, rebased on 3.4 branch.

alalek

Thank you for update!

alalek · 2021-03-10T07:28:53Z

modules/dnn/src/layers/slice_layer.cpp

                        outputs[i][j] = normalize_axis_range(sliceRanges[i][j], inpShape[j]).size();
+
+                    if (! sliceSteps.empty() && sliceSteps[i][j] > 1)
+                        outputs[i][j] = (outputs[i][j] + sliceSteps[i][j] - 1) / sliceSteps[i][j];


Please add out of range checks for sliceSteps[i][j] access.

@alalek, checks were added.

alalek · 2021-03-10T07:59:33Z

modules/dnn/src/layers/slice_layer.cpp

+            if (dim + 1 < dimsNum)
+                getSliceRecursive(inpMat, inpIdx, sliceRanges, sliceSteps, dim + 1, dimsNum, outputs, outIdx);
+            else
+                outputs.at<float>(outIdx.data()) = inpMat.at<float>(inpIdx.data());


IMHO, provided implementation is really slow.

I believe we should have optimized case for 2D (the last 2 dims) processing at least.

@alalek, yes, but if it's ok with you, let's separate this optimization activity and implement it not in this scope.

alalek · 2021-03-10T09:24:29Z

modules/dnn/src/onnx/onnx_importer.cpp

+                    DictValue steps_dict = layerParams.get("steps");
+                    for (int i = 0; i < steps_dict.size(); ++i)
                    {
-                        if (steps.get<int>(i) != 1)
+                        if (steps_dict.get<int>(i) != 1) {
                            CV_Error(Error::StsNotImplemented,
                                "Slice layer only supports steps = 1");


Do we want to support this case?

@alalek, this case could be useful for opset9, which doesn't support step!=1, however, it means that initially onnx model could not be generated with step value != 1, hence, this check is useless.
Just in case, I've checked the model from the issue (which relates to the PR, where this code block was added), there is no step parameter (in opset9 slice layer "no step parameter" => step=1). Removed this code block.

alalek · 2021-03-10T09:28:55Z

modules/dnn/test/test_onnx_importer.cpp

+        if (backend == DNN_BACKEND_OPENCV)
+            applyTestTag(target == DNN_TARGET_OPENCL ? CV_TEST_TAG_DNN_SKIP_OPENCL : CV_TEST_TAG_DNN_SKIP_OPENCL_FP16);


Implementation should fallback on CPU code instead of test skipping.

@alalek, corrected.

OlivierLDff · 2021-03-26T11:22:30Z

Hi, is there any plan to merge this in master?
I merged it in my fork to try to load yolov5 dnn. This work but i have different result in i run the inference on cpu or with cuda backend.
Have you tried CUDA backend with this and yolov5 model?
The result are coherent with OPENCV backend, but with CUDA result are a bit "off" and are scaled x2.
Should i open an issue, or is this something known?
Thanks for the great work.

alalek · 2021-03-26T11:33:16Z

This patch will be merged to master, but without the CUDA part (CUDA code path will fallback on CPU - it works, but slow)

OlivierLDff · 2021-03-26T12:29:59Z

Ok, is there any issue i can follow about this?
Why no CUDA? is there any plan? As you said CPU backend doesn't fit real time application.

alalek · 2021-03-26T16:43:12Z

Fallback CUDA-CPU-CUDA is slow due to data transfers and synchronization points.

Currently CUDA optimizations are maintained by community, so feel free to propose a patch after the merge to master (wait for "(4.x) Merge 3.4" regular PR).

This was referenced Feb 16, 2021

Added Steps support in DNN Slice layer opencv/opencv_extra#854

Merged

ONNX implementation does not support slice layer with steps != 1 #18920

Closed

asmorkalov requested review from dkurt and sl-sergei February 17, 2021 06:39

sl-sergei approved these changes Feb 20, 2021

View reviewed changes

alalek assigned sl-sergei and unassigned sl-sergei Feb 20, 2021

alalek reviewed Feb 20, 2021

View reviewed changes

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

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

dkurt reviewed Feb 24, 2021

View reviewed changes

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

LupusSanctus force-pushed the am/slice_steps branch 2 times, most recently from 96eaa99 to af88ecf Compare February 28, 2021 21:51

LupusSanctus force-pushed the am/slice_steps branch from af88ecf to 105dfb0 Compare March 9, 2021 20:41

LupusSanctus changed the base branch from master to 3.4 March 9, 2021 20:41

alalek reviewed Mar 10, 2021

View reviewed changes

Anastasia Murzova added 2 commits March 24, 2021 23:02

Added Steps support in DNN Slice layer

4fa115c

Added code corrections

303fcbc

LupusSanctus force-pushed the am/slice_steps branch from 105dfb0 to 303fcbc Compare March 25, 2021 20:15

dnn(slice): fix OCL and OCL_FP16 processing

626eaef

alalek approved these changes Mar 26, 2021

View reviewed changes

alalek assigned sl-sergei Mar 26, 2021

alalek merged commit 3e48a91 into opencv:3.4 Mar 26, 2021

alalek mentioned this pull request Mar 27, 2021

(4.x) Merge 3.4 #19796

Merged

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #19885

Merged

OlivierLDff mentioned this pull request Apr 20, 2021

Deploy with OpenCV DNN module ultralytics/yolov5#1043

Closed

		if (backend == DNN_BACKEND_OPENCV)
		applyTestTag(target == DNN_TARGET_OPENCL ? CV_TEST_TAG_DNN_SKIP_OPENCL : CV_TEST_TAG_DNN_SKIP_OPENCL_FP16);

Uh oh!

Conversation

LupusSanctus commented Feb 16, 2021 • 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!

LupusSanctus commented Mar 3, 2021

Uh oh!

alalek commented Mar 3, 2021

Uh oh!

asmorkalov commented Mar 9, 2021

Uh oh!

LupusSanctus commented Mar 9, 2021

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

alalek Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

LupusSanctus Mar 25, 2021

Choose a reason for hiding this comment

Uh oh!

alalek Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

LupusSanctus Mar 25, 2021

Choose a reason for hiding this comment

Uh oh!

alalek Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

LupusSanctus Mar 25, 2021 • 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!

alalek Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

LupusSanctus Mar 25, 2021

Choose a reason for hiding this comment

Uh oh!

OlivierLDff commented Mar 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alalek commented Mar 26, 2021

Uh oh!

OlivierLDff commented Mar 26, 2021

Uh oh!

alalek commented Mar 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

LupusSanctus commented Feb 16, 2021 •

edited by alalek

Loading

LupusSanctus Mar 25, 2021 •

edited

Loading

OlivierLDff commented Mar 26, 2021 •

edited

Loading