dnn: improve logging, catch import errors by alalek · Pull Request #21322 · opencv/opencv

alalek · 2021-12-23T02:09:00Z

relates #19347 (debug helper for the issue)
relates #18145 (incomplete dynamic shapes support)

force_builders=Custom,Custom Win,Custom Mac

build_image:Custom=ubuntu-openvino-2021.4.0:20.04
Xbuild_image:Custom=ubuntu-openvino-2021.3.0:20.04
Xbuild_image:Custom=ubuntu-openvino-2021.2.0:20.04
Xbuild_image:Custom=ubuntu-openvino-2020.3.0:16.04
build_image:Custom Win=openvino-2021.4.2
Xbuild_image:Custom Win=openvino-2021.3.0-vc16
build_image:Custom Mac=openvino-2021.4.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
test_opencl:Custom=ON
test_bigdata:Custom=1
test_filter:Custom=*

buildworker:Custom Win=windows-3
test_opencl:Custom Win=ON
test_bigdata:Custom Win=1
test_filter:Custom Win=*

allow_multiple_commits=1

alalek · 2021-12-23T08:36:20Z

modules/dnn/src/onnx/onnx_importer.cpp

    static const DispatchMap buildDispatchMap();
+
+    int onnx_opset;  // OperatorSetIdProto for 'onnx' domain
+    void parseOperatorSet();


@rogday Added ONNX opset field here.

alalek · 2021-12-23T08:36:22Z

modules/dnn/src/onnx/onnx_importer.cpp

+        else
+        {
+            // OpenCV don't know other opsets
+            CV_LOG_WARNING(NULL, "DNN/ONNX: unsupported opset[" << i << "]: name='" << domain << "' version=" << version);


should we emit error here?

I think we should change the condition to domain.empty() || domain == "ai.onnx" and emit the error if running normally, otherwise we should keep going(if DNN_DIAGNOSTICS_RUN is true).

It is 3.4 branch, there is no DNN_DIAGNOSTICS_RUN

alalek · 2021-12-23T08:37:02Z

modules/dnn/src/onnx/onnx_importer.cpp

+        CV_LOG_WARNING(NULL, "DNN/ONNX: can't handle node with " << node_proto.input_size() << " inputs and " << node_proto.output_size() << " outputs: "
+                << cv::format("[%s@%s]:(%s)", layer_type.c_str(), layer_type_domain.c_str(), name.c_str())
+        );
+        CV_Error(Error::StsNotImplemented, cv::format("ONNX: unsupported domain: %s", layer_type_domain.c_str()));


Added error for non-ONNX domain.

We might want to skip CV_Error if DNN_DIAGNOSTICS_RUN is true.

alalek · 2021-12-23T08:38:46Z

modules/dnn/src/onnx/onnx_importer.cpp

        layerParams.set("mode", "opencv_linear");

-    // input = [X, scales], [X, roi, scales] or [x, roi, scales, sizes]
-    int foundScaleId = hasDynamicShapes ? node_proto.input_size() - 1


avoid using of hasDynamicShapes checks.
Such checks should be eliminated.

rogday

Thank you for contribution!

rogday · 2021-12-23T14:05:01Z

modules/dnn/src/onnx/onnx_importer.cpp


+void ONNXImporter::parseOperatorSet()
+{
+    int ir_version = model_proto.has_ir_version() ? (int)model_proto.ir_version() : -1;


(int) -> static_cast() ?

rogday · 2021-12-23T14:20:52Z

modules/dnn/src/onnx/onnx_importer.cpp

+        else
+        {
+            // OpenCV don't know other opsets
+            CV_LOG_WARNING(NULL, "DNN/ONNX: unsupported opset[" << i << "]: name='" << domain << "' version=" << version);


I think we should change the condition to domain.empty() || domain == "ai.onnx" and emit the error if running normally, otherwise we should keep going(if DNN_DIAGNOSTICS_RUN is true).

rogday · 2021-12-23T14:24:01Z

modules/dnn/src/onnx/onnx_importer.cpp

+        CV_LOG_WARNING(NULL, "DNN/ONNX: can't handle node with " << node_proto.input_size() << " inputs and " << node_proto.output_size() << " outputs: "
+                << cv::format("[%s@%s]:(%s)", layer_type.c_str(), layer_type_domain.c_str(), name.c_str())
+        );
+        CV_Error(Error::StsNotImplemented, cv::format("ONNX: unsupported domain: %s", layer_type_domain.c_str()));


We might want to skip CV_Error if DNN_DIAGNOSTICS_RUN is true.

rogday · 2021-12-23T14:24:24Z

modules/dnn/src/onnx/onnx_importer.cpp

-    for (int j = 0; j < inpShape.size(); ++j)
-        shapeMat.at<int>(j) = inpShape[j];
+    bool isDynamicShape = false;
+    for (int j = 0; j < (int)inpShape.size(); ++j)


static_cast?

rogday · 2021-12-23T14:30:59Z

modules/dnn/src/onnx/onnx_importer.cpp

+    // opset-11: input = [X, roi, scales] or [x, roi, scales, sizes]
+    int foundScaleId = node_proto.input_size() == 2 ? 1 : 2;

    Mat scales = getBlob(node_proto, foundScaleId);


Won't getBlob fail, if it doesn't find scales blob?
The condition below seems fragile.

scales can be empty. getBlob() doesn't fail in that case.

https://github.com/onnx/onnx/blob/master/docs/Operators.md#Resize

The number of elements of 'scales' should be the same as the rank of input 'X'. One of 'scales' and 'sizes' MUST be specified and it is an error if both are specified. If 'sizes' is needed, the user can use an empty string as the name of 'scales' in this operator's input list.

" == 4" condition looks wrong. Reworked for better error messages.

alalek · 2021-12-23T20:08:28Z

👍

alalek mentioned this pull request Dec 23, 2021

Opencv loads models of dynamic input types #19347

Open

4 tasks

alalek commented Dec 23, 2021

View reviewed changes

rogday reviewed Dec 23, 2021

View reviewed changes

alalek added 3 commits December 23, 2021 15:42

dnn: improve debug messages, add ONNX opset version

cc02fcd

dnn(onnx): fix Resize inputs handling

51e65db

dnn(onnx): emit error in Shape for dynamic input

88a18c8

opencv-pushbot merged commit f558944 into opencv:3.4 Dec 23, 2021

This was referenced Dec 24, 2021

(4.x) Merge 3.4 #21340

Merged

(5.x) Merge 4.x #21371

Merged

alalek mentioned this pull request Feb 22, 2022

(5.x) Merge 4.x #21651

Merged

Uh oh!

Conversation

alalek commented Dec 23, 2021 • edited 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!

rogday left a comment

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!

alalek commented Dec 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alalek commented Dec 23, 2021 •

edited

Loading