Skip to content

Fix for index error for Reduce Mean#18845

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
joegeisbauer:fix_reduce_mean_index_error
Nov 23, 2020
Merged

Fix for index error for Reduce Mean#18845
opencv-pushbot merged 1 commit intoopencv:3.4from
joegeisbauer:fix_reduce_mean_index_error

Conversation

@joegeisbauer
Copy link
Copy Markdown
Contributor

@joegeisbauer joegeisbauer commented Nov 18, 2020 •
edited by alalek
Loading

resolves #18898

[ONNX] Fix Reduce Mean Layer Import Error

The onnx_importer.cpp was causing a Segmentation Fault without an error message when converting newer models like MobileNetV2 that had been converted from PyTorch torchvision using torch.onnx. This occurred with the recent update to OpenCV 4.5.0. The culprit was chased down to an indexing issue in the conversion of the ReduceMean layer. You can see the one line change I made in the code comparison of this commit. My understanding was that the intention was to delete the last two dimensions of the tensor which corresponded to the feature size in order to create the correct size tensor for output by changing the last two dimensions. If that is not the expected behavior, then this code will not fix the issue. However, we did verify that this code will fix the issue if that is the expected behavior.

Pull Request Readiness Checklist

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

  • [ X ] I agree to contribute to the project under Apache 2 License.
  • [ X ] To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • [ X ] The PR is proposed to proper branch
  • [ X ] There is reference to original bug report and related work
  • [ X ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • [ X ] The feature is well documented and sample code can be built with the project CMake
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=*

@alalek
Copy link
Copy Markdown
Member

alalek commented Nov 18, 2020

@joegeisbauer Could you please share conversion instructions (with link on original model) and final .onnx file (thrugh GoogleDrive / OneDrive / DropBox), so we can reproduce the problem?

@joegeisbauer
Copy link
Copy Markdown
Contributor Author

joegeisbauer commented Nov 18, 2020 •
edited
Loading

@alalek The models both the state dict checkpoint and the onyx files can be found here. For directions on model conversion, you can check out this link, but I have already converted it for you in the Dropbox folder. The following code was copied from the link

# Input to the model
x = torch.randn(batch_size, 1, 224, 224, requires_grad=True)
torch_out = torch_model(x)

# Export the model
torch.onnx.export(torch_model,               # model being run
                  x,                         # model input (or a tuple for multiple inputs)
                  "super_resolution.onnx",   # where to save the model (can be a file or file-like object)
                  export_params=True,        # store the trained parameter weights inside the model file
                  opset_version=10,          # the ONNX version to export the model to
                  do_constant_folding=True,  # whether to execute constant folding for optimization
                  input_names = ['input'],   # the model's input names
                  output_names = ['output'], # the model's output names
                  dynamic_axes={'input' : {0 : 'batch_size'},    # variable lenght axes
                                'output' : {0 : 'batch_size'}})

@asmorkalov asmorkalov requested a review from sl-sergei November 18, 2020 20:31
sl-sergei
sl-sergei reviewed Nov 19, 2020
View reviewed changes
@sl-sergei
Copy link
Copy Markdown
Contributor

sl-sergei commented Nov 19, 2020

@joegeisbauer Can you please change the target branch from master to 3.4?

@joegeisbauer joegeisbauer changed the base branch from master to 3.4 November 20, 2020 02:30
@asmorkalov asmorkalov changed the base branch from 3.4 to master November 20, 2020 08:13
@asmorkalov asmorkalov added the pr: needs rebase Rebase patch (and squash fixup commits) on the top of target branch label Nov 20, 2020
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Nov 20, 2020

This patch should go into 3.4 branch first. We will merge changes from 3.4 into master regularly (weekly/bi-weekly). 3.4 and master branches are diverged and it's not enough to change target in PR.

So, please:

  • change "base" branch of this PR: master => 3.4 (use "Edit" button near PR title)
  • rebase your commits from master onto 3.4 branch. For example:
    git rebase -i --onto upstream/3.4 upstream/master
    (check list of your commits, save and quit (Esc + "wq" + Enter)
    where upstream is configured by following this GitHub guide and fetched (git fetch upstream).
  • push rebased commits into source branch of your fork (with --force option)

Note: no need to re-open PR, apply changes "inplace".

@joegeisbauer joegeisbauer changed the base branch from master to 3.4 November 20, 2020 16:18
@joegeisbauer
Copy link
Copy Markdown
Contributor Author

joegeisbauer commented Nov 20, 2020

@asmorkalov I took care of the rebase as well. We will see how the builds and testing complete and go from there. Thank you @asmorkalov and @sl-sergei for your patience and understanding on my first ever pull request for OpenCV. It has been a great experience, and makes me want to help by contributing even more as I can.

@joegeisbauer
Fix Reduce Mean error for MobileNets DNN
e05c2e0 
Fix for index error for Reduce Mean

Correct Reduce Mean indexing error
sl-sergei
sl-sergei approved these changes Nov 23, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@sl-sergei sl-sergei 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
Copy link
Copy Markdown
Member

alalek commented Nov 23, 2020

resolves #18898

@opencv-pushbot opencv-pushbot merged commit 0401d59 into opencv:3.4 Nov 23, 2020
@asmorkalov asmorkalov mentioned this pull request Nov 23, 2020
Closed
4 tasks
@joegeisbauer joegeisbauer deleted the fix_reduce_mean_index_error branch November 24, 2020 19:53
This was referenced Nov 27, 2020
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@sl-sergei sl-sergei sl-sergei approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@sl-sergei sl-sergei

Labels

bug category: dnn pr: needs test New functionality requires minimal tests set

Projects

None yet

Milestone

3.4.13

Development

Successfully merging this pull request may close these issues.

Segmentation fault occurs when loading ONNX v6 model shufflenet_v2

5 participants

@joegeisbauer @alalek @sl-sergei @asmorkalov @opencv-pushbot