Skip to content

Fixed bug with constant Div#19372

Merged
alalek merged 2 commits intoopencv:3.4from
l-bat:lb/onnx_pads_calc
Jan 25, 2021
Merged

Fixed bug with constant Div#19372
alalek merged 2 commits intoopencv:3.4from
l-bat:lb/onnx_pads_calc

Conversation

@l-bat
Copy link
Copy Markdown
Contributor

@l-bat l-bat commented Jan 22, 2021

Pull Request Readiness Checklist

resolves: #19359
merge with extra opencv/opencv_extra#837

force_builders=Custom,Custom Win,Custom Mac
build_image:Custom=ubuntu-openvino-2021.1.0:20.04
build_image:Custom Win=openvino-2021.1.0
build_image:Custom Mac=openvino-2021.1.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
# disabled due high memory usage: test_opencl:Custom=ON
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

@l-bat l-bat added this to the 3.4.14 milestone Jan 22, 2021
@l-bat l-bat linked an issue Jan 22, 2021 that may be closed by this pull request
4 tasks
@l-bat l-bat requested a review from dkurt January 22, 2021 10:25
@l-bat l-bat force-pushed the lb/onnx_pads_calc branch from 38b3e17 to 488c73c Compare January 22, 2021 11:29
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Mat inp0 = getBlob(node_proto, 0);
Mat inp1 = getBlob(node_proto, 1);
if (inp0.size != inp1.size && inp1.total() != 1)
CV_Error(Error::StsNotImplemented, "Constant multiply with different shapes");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiply
layer_type == "Mul" || layer_type == "Div"

message is not accurate here.

CV_Error_(Error::StsNotImplemented, ("Different shapes case is not supported with constant inputs: %s", layer_type.c_str()));

Comment on lines +1172 to +1173
Mat out = isDiv ? inp0 / inp1 : inp0.mul(inp1);
out = out.reshape(1, inp0.dims, inp0.size);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to reshape inputs instead? (to avoid failure of operation itself due to strong checks: inp0.size != inp1.size is allowed with .total() == 1)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we can use something like that:

if (isDiv)
  divide(1.0, inp1, inp1);
Mat out = inp0.mul(inp1);

}
}

int axis = layerParams.get<int>("axis", 1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It make sense to add comment with link with description of default axis value (modern opsets doesn't have default): https://github.com/onnx/onnx/blob/master/docs/Changelog.md#Concat-1

Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 👍

@alalek alalek merged commit c12930c into opencv:3.4 Jan 25, 2021
@alalek alalek mentioned this pull request Jan 25, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ONNX: layerId != layer_id.end() in function 'handleNode'

3 participants