Fix issue 22015, let Clip layer support 1-3 inputs by WanliZhong · Pull Request #22100 · opencv/opencv

WanliZhong · 2022-06-14T15:34:25Z

Merge with extra: opencv/opencv_extra#980

This PR will fix the issue #22015

The essence of the issue is that the Clip layer is supposed to accept 1-3 inputs, but the implementation here is based on ReLU6, which only supports attributes as parameters.

opencv/modules/dnn/src/onnx/onnx_importer.cpp

Line 1935 in b19683e

layerParams.type = "ReLU6";

Solution: When parsing the Clip layer, the min and max obtained from node_proto.input(), instead of from layerParams, are passed as parameters to the ReLU6. This allows the Clip layer to support 1-3 inputs.

Let layer Clip support 1-3 inputs.

zihaomu · 2022-06-15T02:20:07Z

Thanks for the contribution @WanliZhong. As a short-term solution I think it's ok. In the long run, it cannot resolve the following Clip node.

added extra checks to min/max handling in Clip

WanliZhong · 2022-06-17T08:04:45Z

Yes, this is a temporary solution. I have added such a model to verify the availability of the clip node when min and max are constant inputs. This will solve the import problem of the image_classification_mobilenetv2_2022apr.onnx model.

alalek

Thank you for contribution!

Please take a look on minor comments below.

alalek · 2022-06-17T08:04:33Z

modules/dnn/src/onnx/onnx_importer.cpp

-    layerParams.set("max_value", layerParams.get<float>("max", FLT_MAX));
+    float min_value = -FLT_MAX, max_value = FLT_MAX;
+    int input_size = node_proto.input_size();
+    CV_Assert(1 <= input_size && input_size <= 3);


CV_Check(input_size, 1 <= input_size && input_size <= 3, "")

is preferable (more clear message in case of problem)

alalek · 2022-06-17T08:07:02Z

modules/dnn/test/test_onnx_importer.cpp

 TEST_P(Test_ONNX_layers, Clip)
 {
    testONNXModels("clip", npy);
+    testONNXModels("clip_init_min_max");
 }


Please create separate test for each model (grouping them is not a good practice).

TEST_P(Test_ONNX_layers, Clip_init_min_max) { testONNXModels("clip_init_min_max"); }

Thanks, I will update the code in the next commit.

…put_size to provide a clearer message in case of problem.

WanliZhong · 2022-06-17T10:04:37Z

@alalek Hi, why doesn't the workflow run automatically after the first approval?

fengyuentau

Other looks good to me 👍

modules/dnn/src/onnx/onnx_importer.cpp

rogday

Thank you!

rogday · 2022-06-21T12:31:58Z

modules/dnn/src/onnx/onnx_importer.cpp

+    if(input_size == 2)
+    {
+        if (constBlobs.find(node_proto.input(1)) != constBlobs.end())
+            min_value = getBlob(node_proto, 1).at<float>(0);
+        else
+            CV_Error(Error::StsNotImplemented, "Non-constant min values in Clip are not supported");
+    }
+
+    if(input_size == 3)
+    {
+        if (constBlobs.find(node_proto.input(1)) != constBlobs.end())
+            min_value = getBlob(node_proto, 1).at<float>(0);
+        if (constBlobs.find(node_proto.input(2)) != constBlobs.end())
+            max_value = getBlob(node_proto, 2).at<float>(0);
+    }


We could do something like:

if(input_size >= 2) { if (constBlobs.find(node_proto.input(1)) != constBlobs.end()) min_value = getBlob(node_proto, 1).at<float>(0); else CV_Error(Error::StsNotImplemented, "Non-constant min values in Clip are not supported"); } if(input_size == 3) { if (constBlobs.find(node_proto.input(2)) != constBlobs.end()) max_value = getBlob(node_proto, 2).at<float>(0); else CV_Error(Error::StsNotImplemented, "Non-constant max values in Clip are not supported"); }

In order to print errors messages in both cases.

Since min and max are optional, I don't think we should throw an error if only min or max is available when the clip is initialized.

This is what I thought would happen:

Optional parameters can either be with empty name or omitted to indicate that they aren't supplied. So, we could just add a check that the name isn't empty.

Thanks! This is indeed a better implementation and I will change it in the next commit.

rogday

LGTM!

rogday · 2022-06-22T12:18:28Z

@alalek, should this be backported to 4.x?

alalek · 2022-06-22T16:38:27Z

It is a feature, so no - if there is no such requirement.

Fix issue 22015, let Clip layer support 1-3 inputs * Fix issue 22015. Let layer Clip support 1-3 inputs. * Resolve other problems caused by modifications * Update onnx_importer.cpp added extra checks to min/max handling in Clip * Add assertions to check the size of the input * Add test for clip with min and max initializers * Separate test for "clip_init_min_max". Change the check method for input_size to provide a clearer message in case of problem. * Add tests for clip with min or max initializers * Change the implementation of getting input Co-authored-by: Vadim Pisarevsky <vadim.pisarevsky@gmail.com>

Fix issue 22015.

0c2e776

Let layer Clip support 1-3 inputs.

zihaomu self-assigned this Jun 15, 2022

WanliZhong and others added 2 commits June 15, 2022 13:18

Resolve other problems caused by modifications

af36fee

Update onnx_importer.cpp

8a9ce3b

added extra checks to min/max handling in Clip

asmorkalov requested a review from rogday June 15, 2022 06:50

Add assertions to check the size of the input

f3448bf

fengyuentau self-requested a review June 15, 2022 08:00

zihaomu removed their assignment Jun 15, 2022

This was referenced Jun 17, 2022

Add model for testing clip layer with min and max initializers opencv/opencv_extra#979

Closed

Add a model for testing clip with min and max initializers opencv/opencv_extra#980

Merged

Add test for clip with min and max initializers

8ca8171

alalek reviewed Jun 17, 2022

View reviewed changes

Separate test for "clip_init_min_max". Change the check method for in…

b8ddfaa

…put_size to provide a clearer message in case of problem.

Add tests for clip with min or max initializers

532977a

fengyuentau requested changes Jun 21, 2022

View reviewed changes

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

fengyuentau approved these changes Jun 21, 2022

View reviewed changes

rogday reviewed Jun 21, 2022

View reviewed changes

Change the implementation of getting input

28ff44a

rogday approved these changes Jun 22, 2022

View reviewed changes

alalek assigned rogday Jun 22, 2022

alalek merged commit a6ca48a into opencv:4.x Jun 22, 2022

This was referenced Jul 5, 2022

image_classification_mobilenet: cannot load models opencv/opencv_zoo#55

Closed

Is it able to provide opencv-python nightly build? opencv/opencv-python#687

Closed

zihaomu mentioned this pull request Aug 15, 2022

cv2.dnn.readNetFromONNX(modelWeights) error #22384

Closed

alalek mentioned this pull request Aug 21, 2022

(5.x) Merge 4.x #22408

Merged

fengyuentau mentioned this pull request Sep 13, 2022

The onnx model was loadable in 4.5.4 but not in 4.6.0 #22139

Closed

WanliZhong mentioned this pull request Mar 14, 2023

ONNX conformance test results #21078

Open

48 tasks

WanliZhong deleted the issue_22015 branch May 16, 2023 12:33

Uh oh!

Conversation

WanliZhong commented Jun 14, 2022 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zihaomu commented Jun 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WanliZhong commented Jun 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

alalek Jun 17, 2022

Choose a reason for hiding this comment

Uh oh!

alalek Jun 17, 2022

Choose a reason for hiding this comment

Uh oh!

WanliZhong Jun 17, 2022

Choose a reason for hiding this comment

Uh oh!

WanliZhong commented Jun 17, 2022

Uh oh!

fengyuentau left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rogday left a comment

Choose a reason for hiding this comment

Uh oh!

rogday Jun 21, 2022

Choose a reason for hiding this comment

Uh oh!

WanliZhong Jun 22, 2022 • 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!

rogday Jun 22, 2022

Choose a reason for hiding this comment

Uh oh!

WanliZhong Jun 22, 2022

Choose a reason for hiding this comment

Uh oh!

rogday left a comment

Choose a reason for hiding this comment

Uh oh!

rogday commented Jun 22, 2022

Uh oh!

alalek commented Jun 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

WanliZhong commented Jun 14, 2022 •

edited by alalek

Loading

zihaomu commented Jun 15, 2022 •

edited

Loading

WanliZhong commented Jun 17, 2022 •

edited

Loading

WanliZhong Jun 22, 2022 •

edited

Loading