dnn: expand refactor with cv::broadcast for onnx models #24295

fengyuentau · 2023-09-19T11:19:40Z

Resolves #24300
Resolves #24308

Motivation:

Current implementation of ONNX Expand is very limited and hard to maintain. It can go very unefficient, e.g. input of shape [1, 5, 1] and shape of value [1, 5, 256], it leads to 256 x [1, 5, 1] constant nodes along with a concat node. See down below for more details .

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

Lines 2485 to 2494 in b870ad4

    
           else if (broadcast_axes.size() == 1) 
        
           { 
        
               // FIXME: this will end up creating massive amount of Identity nodes for broadcasting, 
        
               //        for example, broadcast 1 to 256 needs 256 Identity nodes and 1 Concat node. 
        
               //        Possible improvement is to use "Scale". 
        
               expandMid(layerParams.name, node_proto, srcName, targetShape[broadcast_axes[0]]); 
        
               layerParams.set("axis", broadcast_axes[0]); 
        
               layerParams.type = "Concat"; 
        
               node_proto.set_output(0, output_name);

Since now we have cv::broadcast already, it is time to do a refactor on expand.

Checklist:

handle all-input-constant case
fix expand_neg_batch.onnx
take care of 0d/1d tensor

Pull Request Readiness Checklist

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

asmorkalov · 2023-09-21T06:09:04Z

@fengyuentau Could the patch unlock some "conformance" tests for ONNX?

fengyuentau · 2023-09-21T06:58:19Z

@asmorkalov Unfortunately, all Expand conformance tests have input shape as non-constant, which cannot be supported in the current dnn engine. However, I can still convert all of them with constant shape. Let me do it.

…ues; add early copy for unchanged total elements

fengyuentau · 2023-09-21T10:16:58Z

CI will not pass due to #24308. ~~Need to fix the problem in another PR first.~~

Decided to fix the issue here in this pull request. Basically the wrong result is given because of shape is used as input shape for Expand, which has value [2, -1, -1, -1] (See line 838):

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

Lines 824 to 840 in 3889dcf

    
           class ExpandSubgraph : public Subgraph 
        
           { 
        
           public: 
        
               ExpandSubgraph() 
        
               { 
        
                   int input = addNodeToMatch(""); 
        
                   int values = addNodeToMatch(""); 
        
                   int init = addNodeToMatch("ConstantOfShape", values); 
        
                   int coeff = addNodeToMatch("Constant"); 
        
                   int mul = addNodeToMatch("Mul", init, coeff); 
        
                   int shape = addNodeToMatch("Constant"); 
        
                   int condition = addNodeToMatch("Equal", shape, mul); 
        
                   int where = addNodeToMatch("Where", condition, init, addNodeToMatch("Constant")); 
        
                   addNodeToMatch("Expand", input, where); 
        
                   setFusedNode("Expand", input, shape); 
        
               } 
        
           };

fengyuentau · 2023-09-23T02:40:18Z

CI is not passing again because of #24300

fengyuentau · 2023-09-23T10:01:31Z

All tests are green now.

modules/dnn/test/test_onnx_importer.cpp

* add expand impl with cv::broadcast * remove expandMid * deduce shape from -1 * add constant folding * handle input constant; handle input constant 1d * add expand conformance tests; add checks to disallow shape of neg values; add early copy for unchanged total elements * fix ExpandSubgraph * dummy commit to trigger build * dummy commit to trigger build 1 * remove conformance from test names

fengyuentau added category: dnn category: dnn (onnx) ONNX suport issues in DNN module labels Sep 19, 2023

fengyuentau added this to the 4.9.0 milestone Sep 19, 2023

fengyuentau force-pushed the add_onnx_expand branch from 89929d6 to 4e72c5a Compare September 20, 2023 08:57

fengyuentau added 5 commits September 21, 2023 10:30

add expand impl with cv::broadcast

9842ff5

remove expandMid

e0b64a1

deduce shape from -1

b25fa92

add constant folding

78a05a7

handle input constant; handle input constant 1d

cf0ea25

fengyuentau force-pushed the add_onnx_expand branch from 4e72c5a to cf0ea25 Compare September 21, 2023 02:30

fengyuentau marked this pull request as ready for review September 21, 2023 02:49

fengyuentau requested review from dkurt and rogday September 21, 2023 02:49

fengyuentau mentioned this pull request Sep 21, 2023

dnn: wrong internal results in expand_neg_batch in Test_ONNX_layers.Expand #24308

Closed

4 tasks

add expand conformance tests; add checks to disallow shape of neg val…

6c9d366

…ues; add early copy for unchanged total elements

fix ExpandSubgraph

e2d7ec0

fengyuentau mentioned this pull request Sep 23, 2023

add expand conformance tests opencv/opencv_extra#1098

Merged

dummy commit to trigger build

778a5c4

dummy commit to trigger build 1

114c72a

opencv-alalek reviewed Sep 23, 2023

View reviewed changes

modules/dnn/test/test_onnx_importer.cpp Outdated Show resolved Hide resolved

remove conformance from test names

c4ada39

vpisarev self-requested a review September 27, 2023 06:27

vpisarev approved these changes Sep 27, 2023

View reviewed changes

vpisarev merged commit bb171a0 into opencv:4.x Sep 27, 2023

fengyuentau mentioned this pull request Sep 27, 2023

Update OpenVINO init of new GEMM layer #24309

Merged

8 tasks

fengyuentau deleted the add_onnx_expand branch September 27, 2023 07:02

asmorkalov mentioned this pull request Sep 28, 2023

(5.x) Merge 4.x #24338

Merged

fengyuentau mentioned this pull request Feb 21, 2024

ONNX conformance test results #21078

Open

48 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

dnn: expand refactor with cv::broadcast for onnx models #24295

dnn: expand refactor with cv::broadcast for onnx models #24295

Uh oh!

fengyuentau commented Sep 19, 2023 •

edited

Loading

Uh oh!

asmorkalov commented Sep 21, 2023

Uh oh!

fengyuentau commented Sep 21, 2023

Uh oh!

fengyuentau commented Sep 21, 2023 •

edited

Loading

Uh oh!

fengyuentau commented Sep 23, 2023

Uh oh!

fengyuentau commented Sep 23, 2023

Uh oh!

Uh oh!

Uh oh!

	else if (broadcast_axes.size() == 1)
	{
	// FIXME: this will end up creating massive amount of Identity nodes for broadcasting,
	// for example, broadcast 1 to 256 needs 256 Identity nodes and 1 Concat node.
	// Possible improvement is to use "Scale".
	expandMid(layerParams.name, node_proto, srcName, targetShape[broadcast_axes[0]]);

	layerParams.set("axis", broadcast_axes[0]);
	layerParams.type = "Concat";
	node_proto.set_output(0, output_name);

Uh oh!

dnn: expand refactor with cv::broadcast for onnx models #24295

dnn: expand refactor with cv::broadcast for onnx models #24295

Uh oh!

Conversation

fengyuentau commented Sep 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

asmorkalov commented Sep 21, 2023

Uh oh!

fengyuentau commented Sep 21, 2023

Uh oh!

fengyuentau commented Sep 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fengyuentau commented Sep 23, 2023

Uh oh!

fengyuentau commented Sep 23, 2023

Uh oh!

Uh oh!

Uh oh!

fengyuentau commented Sep 19, 2023 •

edited

Loading

fengyuentau commented Sep 21, 2023 •

edited

Loading