dnn onnx: fix for multiple Gather operators with shared constants. by fengyuentau · Pull Request #24334 · opencv/opencv

fengyuentau · 2023-09-28T04:05:27Z

Resolves #24319
Merge with opencv/opencv_extra#1101

Problem

The model in the issue above has several Gather operators which have shared constant indices (same name, same category Constant, same type, same value). The following piece of code in onnx importer adds an attribute real_ndims if indices is not a layer

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

Lines 2506 to 2531 in 7b399c4

    
           if (layer_id.find(node_proto.input(1)) == layer_id.end()) 
        
           { 
        
               int real_ndims = getBlobExtraInfo(node_proto.input(1)).real_ndims; 
        
               layerParams.set("real_ndims", real_ndims); 
        
               if (layer_id.find(node_proto.input(0)) == layer_id.end()) 
        
               { 
        
                   std::vector<Mat> inputs, output; 
        
                   Mat input = getBlob(node_proto, 0); 
        
                   int input_real_ndims = input.dims; 
        
                   int type = input.type(); 
        
                   input.convertTo(input, CV_32FC1); 
        
                   inputs.push_back(input); 
        
                   Mat indices = getBlob(node_proto, 1); 
        
                   indices.convertTo(indices, CV_32FC1); 
        
                   inputs.push_back(indices); 
        
                   runLayer(layerParams, inputs, output); 
        
                   output.back().convertTo(output.back(), type); 
        
                   if (real_ndims < 2)  // In case of scalars or 1D vectors, OpenCV initializes 2D cv::Mat 
        
                       output.back().dims = std::max(input_real_ndims - real_ndims, 1); 
        
                   addConstant(node_proto.output(0), output.back()); 
        
                   return; 
        
               } 
        
           }

, but the following code converts the constant indices into a Const layer:

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

Lines 2533 to 2551 in 7b399c4

    
           for (int i = 0; i < node_proto.input_size(); ++i) 
        
           { 
        
               if (layer_id.find(node_proto.input(i)) == layer_id.end()) 
        
               { 
        
                   LayerParams constParams; 
        
                   constParams.name = node_proto.input(i); 
        
                   constParams.type = "Const"; 
        
                   Mat blob = getBlob(node_proto, i); 
        
                   if (i == 1) 
        
                   { 
        
                       blob.convertTo(blob, CV_32FC1); 
        
                   } 
        
                   constParams.blobs.push_back(blob); 
        
                   opencv_onnx::NodeProto proto; 
        
                   proto.add_output(constParams.name); 
        
                   addLayer(constParams, proto); 
        
               } 
        
           }

, which makes problems for later Gather operators who have shared indices. For later Gather who have shared indices, indices have been converted to a Const layer in dnn, leading to real_ndims non-set.

Similar problems may happen on other operators as well. Need further investigation.

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

dkurt

Thank you!

init commit

7fa0493

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

fengyuentau requested a review from dkurt September 28, 2023 04:05

fengyuentau added this to the 4.9.0 milestone Sep 28, 2023

This was referenced Sep 28, 2023

error loading onnx model while reading it from opencv dnn module #24319

Closed

Model and test data for the fix of shared const inputs opencv/opencv_extra#1101

Merged

dkurt approved these changes Sep 28, 2023

View reviewed changes

dkurt self-assigned this Sep 28, 2023

asmorkalov merged commit b8d4ac5 into opencv:4.x Sep 28, 2023

asmorkalov mentioned this pull request Sep 28, 2023

(5.x) Merge 4.x #24338

Merged

fengyuentau deleted the fix_24319 branch September 29, 2023 08:28

fengyuentau changed the title ~~dnn onnx: fix not-found constant indices for Gather if shared~~ dnn onnx: fix for multiple Gather operators with shared constants. Oct 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

dnn onnx: fix for multiple Gather operators with shared constants.#24334

dnn onnx: fix for multiple Gather operators with shared constants.#24334
asmorkalov merged 1 commit intoopencv:4.xfrom
fengyuentau:fix_24319

fengyuentau commented Sep 28, 2023 •

edited by asmorkalov

Loading

Uh oh!

dkurt left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if (layer_id.find(node_proto.input(1)) == layer_id.end())
	{
	int real_ndims = getBlobExtraInfo(node_proto.input(1)).real_ndims;
	layerParams.set("real_ndims", real_ndims);
	if (layer_id.find(node_proto.input(0)) == layer_id.end())
	{
	std::vector<Mat> inputs, output;

	Mat input = getBlob(node_proto, 0);
	int input_real_ndims = input.dims;
	int type = input.type();
	input.convertTo(input, CV_32FC1);
	inputs.push_back(input);

	Mat indices = getBlob(node_proto, 1);
	indices.convertTo(indices, CV_32FC1);
	inputs.push_back(indices);

	runLayer(layerParams, inputs, output);
	output.back().convertTo(output.back(), type);
	if (real_ndims < 2) // In case of scalars or 1D vectors, OpenCV initializes 2D cv::Mat
	output.back().dims = std::max(input_real_ndims - real_ndims, 1);
	addConstant(node_proto.output(0), output.back());
	return;
	}
	}

	for (int i = 0; i < node_proto.input_size(); ++i)
	{
	if (layer_id.find(node_proto.input(i)) == layer_id.end())
	{
	LayerParams constParams;
	constParams.name = node_proto.input(i);
	constParams.type = "Const";
	Mat blob = getBlob(node_proto, i);
	if (i == 1)
	{
	blob.convertTo(blob, CV_32FC1);
	}
	constParams.blobs.push_back(blob);

	opencv_onnx::NodeProto proto;
	proto.add_output(constParams.name);
	addLayer(constParams, proto);
	}
	}

Uh oh!

Conversation

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

Uh oh!

Problem

Pull Request Readiness Checklist

Uh oh!

dkurt left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fengyuentau commented Sep 28, 2023 •

edited by asmorkalov

Loading