Instancenorm onnx by dvd42 · Pull Request #14858 · opencv/opencv

dvd42 · 2019-06-21T15:14:43Z

Merge with extra: opencv/opencv_extra#627

This Pull request is an implementation of the InstaceNorm operation for the ONNX importer. I am opening this draft PR with the hopes of getting some reviewers to look at the issue I ve been having with it.

The first Issue is that I use the cv::meanStdDev function from OpenCv inside the layer, this operation messes up the Std computation, making it differ from the one computed by the one in my test model (implemented in PyTorch). Making it fail the normAssert test in my test case.

The second issue is that this implementation seems to break the MVN layer test for some Caffe models. Which is very strange given the fact that I have not touched that layer implementation.

The implementation seems to be correct except for this precision issue, so I am looking for some help regarding both these issues.

PS: The only reason I am opening the PR and not a separate issue is because this way the reviewers can easily see all the files that I have changed.

**WIP**
force_builders=Custom,Custom Win,Custom Mac
build_image:Custom=ubuntu-openvino-2019r1:16.04
build_image:Custom Win=openvino-2019r1
build_image:Custom Mac=openvino-2019r1

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
test_opencl:Custom=ON
test_bigdata:Custom=1
test_filter:Custom=*

modules/dnn/src/onnx/onnx_importer.cpp

modules/dnn/test/test_onnx_importer.cpp

alalek · 2019-06-27T19:57:08Z

This patch should go into 3.4 branch first. We will merge changes from 3.4 into master regularly (weekly/bi-weekly).

The same note is about opencv_extra's 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 needs to re-open PR, apply changes "inplace".

This reverts commit 1a60a37.

dvd42 · 2019-06-27T20:23:02Z

Hey @alalek Its done, thanks for your clear instructions! :). I assume this procedure must be done every time I open a PR, just in case 3.4 branch has changed, yes?

alalek · 2019-06-27T20:37:08Z

just in case 3.4 branch has changed

Only if conflicts observed and automatic merge is not possible.

Main problem here is to select "target branch for the patch" initially.
Any patches which can be applied onto 3.4 branch should go there first (bug fixes, additions, etc) while there is regular merges into master ("Merge-3.4" PRs).

dkurt · 2019-06-28T15:15:07Z

@dvd42, I were able to use the following code to pass your test case:

        else if (layer_type == "InstanceNormalization")
        {
            // InstanceNormalization = MVN + BatchNorm
            {
                LayerParams mvnLP;
                mvnLP.name = layerParams.name + "/mvn";
                mvnLP.type = "MVN";
                mvnLP.set("eps", layerParams.get<float>("epsilon"));
                layerParams.erase("epsilon");

                int id = dstNet.addLayer(mvnLP.name, mvnLP.type, mvnLP);

                // Connect to input
                layerId = layer_id.find(node_proto.input(0));
                CV_Assert(layerId != layer_id.end());
                dstNet.connect(layerId->second.layerId, layerId->second.outputId, id, 0);

                // Add shape
                layer_id.insert(std::make_pair(mvnLP.name, LayerInfo(id, 0)));
                outShapes[mvnLP.name] = outShapes[node_proto.input(0)];

                // Replace Batch Norm's input to MVN
                node_proto.set_input(0, mvnLP.name);
            }

            layerParams.type = "BatchNorm";
            Mat mean(3, 1, CV_32F, Scalar(0));
            Mat std(3, 1, CV_32F, Scalar(1));
            Mat weights = getBlob(node_proto, constBlobs, 1);
            Mat bias =  getBlob(node_proto, constBlobs, 2);

            layerParams.blobs.resize(4);
            layerParams.blobs[0] = mean;
            layerParams.blobs[1] = std;
            layerParams.blobs[2] = weights;
            layerParams.blobs[3] = bias;
            layerParams.set("has_bias", true);
            layerParams.set("has_weight", true);
        }

Please check it and try to find the difference with your implementation.

modules/dnn/src/onnx/onnx_importer.cpp

dkurt · 2019-07-02T05:27:00Z

modules/dnn/src/onnx/onnx_importer.cpp

+
+            layerParams.blobs.resize(2);
+
+            if (!node_proto.input(1).empty()) {


If node_proto.input_size() == 3 can we have an empty input name?

In my experience no. However the same thing happens for the BatchNorm layer, and I was just trying to follow the same convention, also felt like an extra sanity check wouldnt hurt. Let me know if I should remove it

Let's assume that they aren't empty.

@dkurt, should be fixed now

dkurt · 2019-07-02T05:28:18Z

modules/dnn/src/onnx/onnx_importer.cpp

+            layerId = layer_id.find(node_proto.input(0));
+            CV_Assert(layerId != layer_id.end());
+            dstNet.connect(layerId->second.layerId, layerId->second.outputId, id, 0);
+            shapeIt = outShapes.find(node_proto.input(0));


Why it's not used?

dkurt · 2019-07-02T05:28:38Z

modules/dnn/src/onnx/onnx_importer.cpp

+            node_proto.set_input(0, mvnParams.name);
+
+            layerParams.type = "BatchNorm";
+


please avoid extra changes here and below (one empty line added and one removed)

modules/dnn/src/onnx/onnx_importer.cpp

modules/dnn/test/test_onnx_importer.cpp

dkurt

👍 Thank you!

Instancenorm onnx (opencv#14858) * Onnx unsupported operation handling * instance norm implementation * Revert "Onnx unsupported operation handling" * instance norm layer test * onnx instancenorm layer

dvd42 force-pushed the instancenorm_onnx branch from d6b4d0b to a4e67e1 Compare June 21, 2019 15:26

dkurt reviewed Jun 22, 2019

View reviewed changes

modules/dnn/src/onnx/onnx_importer.cpp Outdated Show resolved Hide resolved

dkurt added the category: dnn label Jun 22, 2019

dkurt reviewed Jun 22, 2019

View reviewed changes

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

dvd42 force-pushed the instancenorm_onnx branch from 43219f0 to d3ab90f Compare June 25, 2019 15:13

This was referenced Jun 26, 2019

Clip Operation for ONNX #14902

Closed

onnx instanceNorm layer test data opencv/opencv_extra#627

Merged

dvd42 changed the base branch from master to 3.4 June 27, 2019 20:04

dvd42 added 4 commits June 27, 2019 22:06

Onnx unsupported operation handling

9c04603

Revert "Onnx unsupported operation handling"

eafe1af

This reverts commit 1a60a37.

instance norm implementation

44f4833

instance norm layer test

ffe4fda

dvd42 force-pushed the instancenorm_onnx branch from d3ab90f to 6840367 Compare June 27, 2019 20:09

dvd42 force-pushed the instancenorm_onnx branch from 6110948 to 32b49eb Compare July 1, 2019 08:29

dkurt reviewed Jul 1, 2019

View reviewed changes

modules/dnn/src/onnx/onnx_importer.cpp Outdated Show resolved Hide resolved

dvd42 force-pushed the instancenorm_onnx branch from 9092ebe to 3022b19 Compare July 1, 2019 13:05

dkurt reviewed Jul 2, 2019

View reviewed changes

dvd42 force-pushed the instancenorm_onnx branch 3 times, most recently from b518adf to c98f1da Compare July 2, 2019 08:37

dkurt reviewed Jul 3, 2019

View reviewed changes

modules/dnn/src/onnx/onnx_importer.cpp Show resolved Hide resolved

dkurt reviewed Jul 3, 2019

View reviewed changes

modules/dnn/src/onnx/onnx_importer.cpp Outdated Show resolved Hide resolved

dvd42 force-pushed the instancenorm_onnx branch 2 times, most recently from bd43273 to 7661f80 Compare July 4, 2019 11:06

dkurt reviewed Jul 4, 2019

View reviewed changes

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

onnx instancenorm layer

3e44d41

dvd42 force-pushed the instancenorm_onnx branch from 2ee95b5 to 3e44d41 Compare July 4, 2019 14:14

dkurt approved these changes Jul 4, 2019

View reviewed changes

alalek assigned dkurt Jul 4, 2019

alalek merged commit 57fae4a into opencv:3.4 Jul 4, 2019

alalek mentioned this pull request Jul 9, 2019

Merge 3.4 #15004

Merged

dkurt mentioned this pull request Jul 17, 2019

Failure Exporting Onnx Models #14984

Closed


		layerParams.blobs.resize(2);

		if (!node_proto.input(1).empty()) {

		node_proto.set_input(0, mvnParams.name);

		layerParams.type = "BatchNorm";

Uh oh!

Conversation

dvd42 commented Jun 21, 2019 • edited by dkurt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alalek commented Jun 27, 2019

Uh oh!

dvd42 commented Jun 27, 2019

Uh oh!

alalek commented Jun 27, 2019

Uh oh!

dkurt commented Jun 28, 2019

Uh oh!

Uh oh!

dkurt Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

dvd42 Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

dkurt Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

dvd42 Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

dkurt Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

dvd42 Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

dkurt Jul 2, 2019 • 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!

dvd42 Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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

dvd42 commented Jun 21, 2019 •

edited by dkurt

Loading

dkurt Jul 2, 2019 •

edited

Loading