Replace Slice optional inputs removal to adjustment by dkurt · Pull Request #24672 · opencv/opencv

dkurt · 2023-12-08T20:30:40Z

Pull Request Readiness Checklist

resolves #24671

TODO:

Test with attention layer PR for which initial subgraph has been added.

patch

--- a/modules/dnn/src/onnx/onnx_graph_simplifier.cpp
+++ b/modules/dnn/src/onnx/onnx_graph_simplifier.cpp
@@ -313,18 +313,30 @@ class AttentionSubGraph : public Subgraph {
         att_add = addNodeToMatch("Add", addNodeToMatch(""), att_matmul);
 
         // v_path
-        slice_v = addNodeToMatch("Slice", att_add, addNodeToMatch(""), addNodeToMatch(""));
+        slice_v = addNodeToMatch("Slice", {att_add,
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch("")});
         int reshape_v = addNodeToMatch("Reshape", slice_v, addNodeToMatch(""));
         int transpose_v = addNodeToMatch("Transpose", reshape_v);
 
         // q_path
-        slice_q = addNodeToMatch("Slice", att_add, addNodeToMatch(""), addNodeToMatch(""));
+        slice_q = addNodeToMatch("Slice", {att_add,
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch("")});
         reshape_q = addNodeToMatch("Reshape", slice_q, addNodeToMatch(""));
         int transpose_q = addNodeToMatch("Transpose", reshape_q);
         div_q = addNodeToMatch("Div", transpose_q, addNodeToMatch(""));
 
         // k_path
-        slice_k = addNodeToMatch("Slice", att_add, addNodeToMatch(""), addNodeToMatch(""));
+        slice_k = addNodeToMatch("Slice", {att_add,
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch("")});
         int reshape_k = addNodeToMatch("Reshape", slice_k, addNodeToMatch(""));
         int transpose_k = addNodeToMatch("Transpose", reshape_k);
 
@@ -422,16 +434,28 @@ class AttentionSingleHeadSubGraph : public Subgraph {
         att_add = addNodeToMatch("Add", addNodeToMatch(""), att_matmul);
 
         // v_path
-        slice_v = addNodeToMatch("Slice", att_add, addNodeToMatch(""), addNodeToMatch(""));
+        slice_v = addNodeToMatch("Slice", {att_add,
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch("")});
         int transpose_v = addNodeToMatch("Transpose", slice_v);
 
         // q_path
-        slice_q = addNodeToMatch("Slice", att_add, addNodeToMatch(""), addNodeToMatch(""));
+        slice_q = addNodeToMatch("Slice", {att_add,
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch("")});
         int transpose_q = addNodeToMatch("Transpose", slice_q);
         div_q = addNodeToMatch("Div", transpose_q, addNodeToMatch(""));
 
         // k_path
-        slice_k = addNodeToMatch("Slice", att_add, addNodeToMatch(""), addNodeToMatch(""));
+        slice_k = addNodeToMatch("Slice", {att_add,
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch(""),
+                                             addNodeToMatch("")});
         int transpose_k = addNodeToMatch("Transpose", slice_k);
 
         // qk

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

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 another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the 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

fengyuentau · 2023-12-09T08:21:36Z

modules/dnn/src/onnx/onnx_graph_simplifier.cpp

+    {
+        opencv_onnx::NodeProto* node = fusedNode.dynamicCast<ONNXNodeWrapper>()->node;
+        for (int i = num_inputs_; i < 5; ++i) {
+            node->add_input("");


Doesn't it lose connections to the existing constants (starts, ends, axes and steps)?

No, It should append missed connections to axes and steps pointing to a fake empty initializer added by

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

Line 71 in 850ebec

net.add_initializer();

How does this appending happen? What I see here is input names are missed already.

For example, node which found with 3 real inputs (data, starts, ends) is appended up to 5 inputs with add_input:

inputs: "data_name", "starts_name", "ends_name", "", ""

net.add_initializer(); adds an empty tensor with an empty name. So the final version of the Slice refers to it.

I have observed such behavior with resize_nearest_unfused_opset11_torch1.4 model:

Yes, I got you point here. But actually my problem is why this

- setFusedNode("Slice", std::vector<int>{input, starts, ends}); + setFusedNode("Slice", inputs);

does not drop the starts and ends constants.

Ahhh, sorry, I missed that. I thought it was input.

@dkurt is there a way for me to check these fixes locally before merging? Wanna make sure if yolox test do pass.

@Abdurrahheem, do the following:

git remote add dkurt https://github.com/dkurt/opencv git fetch dkurt adjust_slice_optional_inputs git checkout dkurt/adjust_slice_optional_inputs

@dkurt is there a way for me to check these fixes locally before merging? Wanna make sure if yolox test do pass.

git cherry-pick should also work right? The simplest way is do the same changes as this PR manually, so you don’t need to mess around with git operations.

@dkurt just checked test with the new patch. Everything related to yolo detectors passes. Thank you for a quick fix!

Replace Slice optional inputs removal to adjustment

ac4b26a

dkurt added the category: dnn (onnx) ONNX suport issues in DNN module label Dec 8, 2023

dkurt requested review from Abdurrahheem and fengyuentau December 8, 2023 20:30

dkurt mentioned this pull request Dec 8, 2023

Yolox ONNX graph parsing error #24671

Closed

4 tasks

opencv-alalek added the bug label Dec 8, 2023

opencv-alalek added this to the 4.9.0 milestone Dec 8, 2023

dkurt mentioned this pull request Dec 9, 2023

dnn: add attention layer #24476

Merged

13 tasks

fengyuentau reviewed Dec 9, 2023

View reviewed changes

fengyuentau approved these changes Dec 10, 2023

View reviewed changes

asmorkalov assigned fengyuentau Dec 10, 2023

asmorkalov merged commit 098efb6 into opencv:4.x Dec 10, 2023

asmorkalov mentioned this pull request Jan 19, 2024

5.x merge 4.x #24862

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace Slice optional inputs removal to adjustment#24672

Replace Slice optional inputs removal to adjustment#24672
asmorkalov merged 1 commit intoopencv:4.xfrom
dkurt:adjust_slice_optional_inputs

dkurt commented Dec 8, 2023 •

edited

Loading

Uh oh!

fengyuentau Dec 9, 2023

Uh oh!

dkurt Dec 9, 2023 •

edited

Loading

Uh oh!

fengyuentau Dec 9, 2023

Uh oh!

dkurt Dec 9, 2023 •

edited

Loading

Uh oh!

fengyuentau Dec 9, 2023

Uh oh!

fengyuentau Dec 9, 2023

Uh oh!

Abdurrahheem Dec 9, 2023 •

edited

Loading

Uh oh!

dkurt Dec 9, 2023

Uh oh!

fengyuentau Dec 10, 2023

Uh oh!

Abdurrahheem Dec 10, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

dkurt commented Dec 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

fengyuentau Dec 9, 2023

Choose a reason for hiding this comment

Uh oh!

dkurt Dec 9, 2023 • 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!

fengyuentau Dec 9, 2023

Choose a reason for hiding this comment

Uh oh!

dkurt Dec 9, 2023 • 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!

fengyuentau Dec 9, 2023

Choose a reason for hiding this comment

Uh oh!

fengyuentau Dec 9, 2023

Choose a reason for hiding this comment

Uh oh!

Abdurrahheem Dec 9, 2023 • 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!

dkurt Dec 9, 2023

Choose a reason for hiding this comment

Uh oh!

fengyuentau Dec 10, 2023

Choose a reason for hiding this comment

Uh oh!

Abdurrahheem Dec 10, 2023 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dkurt commented Dec 8, 2023 •

edited

Loading

dkurt Dec 9, 2023 •

edited

Loading

dkurt Dec 9, 2023 •

edited

Loading

Abdurrahheem Dec 9, 2023 •

edited

Loading

Abdurrahheem Dec 10, 2023 •

edited

Loading