Add {valid, full} option for Pooling layer #3392

Darwin2011 · 2016-09-28T08:17:53Z

Mxnet's pooling layer's output doesn't match Caffe's pooling's output, although Mxnet's convolution layer's output shape is same as Caffe. This issue will cause Googlenet from caffe convertor failing.

The following links shows pooling output size formula.
for caffe,
https://github.com/BVLC/caffe/blob/master/src/caffe/layers/pooling_layer.cpp#L90
for mxnet,
https://github.com/dmlc/mxnet/blob/master/src/operator/pooling-inl.h#L202

And please see #2999. @winstywang @ChenFengAndy

So I add two options for pooling layer {valid, same}. valid is for current mxnet's pooling output and for caffe, it uses same option

default pooling option is valid, same as current mxnet.
if model is from caffe, same option for pooling layer will be on.

Please give some comments. Thanks a lot.

1) default pooling option is valid. 2) if model is from caffe, same option for pooling layer will be on.

sxjscience · 2016-09-28T10:00:56Z

Why call it same?

Darwin2011 · 2016-09-28T10:10:09Z

@sxjscience
Maybe "full" is one better name? I will change it to "full". That makes more sense.

sxjscience · 2016-09-28T10:47:48Z

@Darwin2011 Could we manually adjust the padding to achieve the similar result?

Darwin2011 · 2016-09-28T12:31:52Z

@sxjscience Could you please explain a little bit more? Thanks.

sxjscience · 2016-09-28T12:42:22Z

@Darwin2011 I'm not sure whether this will break the C++ implementations.

sxjscience · 2016-09-29T03:23:13Z

@antinucleon @piiswrong @winstywang What do you think about this? Some old caffemodels like VGG_CNN_M requires this type of compatible pooling. Another solution is to fine-tune these networks using the revised structure (pad 1 in the pooling layer) and add them to our own model zoo.

Darwin2011 · 2016-09-29T06:43:35Z

@sxjscience
Caffe model converter also cannot translate bvlc googlenet on my side.

sxjscience · 2016-09-29T08:25:10Z

@Darwin2011 Yes, this is a problem. One way to deal with the problem is to add pad=(1,1) for lower level pooling layers like here (https://github.com/dmlc/mxnet/blob/master/example/image-classification/symbol_inception-bn-28-small.py#L19-L20). However, we may need to fine-tune the network again since the pooling function has been changed.
BTW, have you tested if the pooling layer works well for the SAME mode?

Darwin2011 · 2016-09-29T09:05:09Z

@sxjscience
I haven't tested it. What I only test is in "same" mode is on, Caffe convertor model can work well and mxnet can get same accuracy as Caffe. Let me test it with cifar10 training.

Thanks.

piiswrong · 2016-09-30T00:03:12Z

I think this should be added. Although the name "same" sounds cryptic. How about "compatible"?

BTW, put kValid first in the enum

piiswrong · 2016-09-30T00:04:33Z

src/operator/pooling-inl.h

@@ -25,13 +25,15 @@ namespace pool_enum {
 enum PoolingOpInputs {kData};
 enum PoolingOpOutputs {kOut};
 enum PoolingOpType {kMaxPooling, kAvgPooling, kSumPooling};
+enum PoolingOpPadConventionType {kSame, kValid};


put kValid first

piiswrong · 2016-09-30T00:09:05Z

src/operator/pooling-inl.h

@@ -47,6 +49,11 @@ struct PoolingParam : public dmlc::Parameter<PoolingParam> {
    .add_enum("avg", pool_enum::kAvgPooling)
    .add_enum("sum", pool_enum::kSumPooling)
    .describe("Pooling type to be applied.");
+
+    DMLC_DECLARE_FIELD(pad_convention).set_default(pool_enum::kValid)
+    .add_enum("same", pool_enum::kSame)


change same to "full" or "compatible" and add document describing the difference (round up or down, discard extra or pad zero) and say that use full for compatibility with caffe

sbodenstein · 2016-09-30T11:16:34Z

@piiswrong @Darwin2011 @sxjscience: These names are really bad. In the literature, 'same', 'valid' and 'full' have precise meanings for pooling and convolutions:

same: the tensor is padded so that the input and output tensors are the same width and height.
valid: no padding

This is used by TensorFlow or Numpy, for example. Using these names for this will cause confusion (esp. if MXnet decided to add these very useful options for convolution + pooling).

One option: have a "caffe_compatibility" (bool) option.

Here is a question that we should answer first: what is the actual computational difference in the pooling layer? Is the MXNet pooling layer equivalent to Caffe with some extra padding for certain input dimensions? If so, we can fix this at the Caffe importer level rather than hacking it into the MXNet pooling definition.

piiswrong · 2016-09-30T16:50:49Z

the difference is basically round down vs up in division. So valid makes
sense. We just need to name the other one better.

On Sep 30, 2016 4:16 AM, "Sebastian Bodenstein" [email protected]
wrote:

@piiswrong https://github.com/piiswrong @Darwin2011
https://github.com/Darwin2011 @sxjscience
https://github.com/sxjscience: These names are really bad. In the
literature, 'same', 'valid' and 'full' have precise meanings for pooling
and convolutions in the literature:

same: the tensor is padded so that the input and output tensors are
the same width and height.

valid: no padding

This is used by TensorFlow
https://www.tensorflow.org/versions/r0.10/api_docs/python/nn.html#convolution
or Numpy
http://docs.scipy.org/doc/numpy/reference/generated/numpy.convolve.html,
for example. Using these names for this will cause confusion (esp. if MXnet
decided to add these very useful options for convolution + pooling).

How about "caffe_compatibibility" which is a bool?

Here is a question that we should answer first: what is the actual
computational difference in the pooling layer? Is the MXNet pooling layer
equivalent to Caffe with some extra padding for certain input dimensions?
If so, we can fix this at the Caffe importer level rather than hacking it
into the MXNet pooling definition.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3392 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAiudGFZW_pavp69Y6cygNwmCsBW_WUnks5qvO-pgaJpZM4KIiS7
.

add some description

Darwin2011 · 2016-10-01T02:59:36Z

@sbodenstein Thanks for your advice.
Please see soumith/convnet-benchmarks#82.
Googlenet symbols fails because of different pooling output size conventions. That's why I add one more options to pooling layer.

Darwin2011 · 2016-10-01T03:01:15Z

Thanks a lot for your review, @piiswrong
renaming done and add some descriptions.

sxjscience · 2016-10-01T07:09:47Z

@Darwin2011 I think we can merge this in if the test passes.

piiswrong · 2016-10-03T22:03:13Z

please update

Add {valid, same} option for Pooling layer

0a7da2a

1) default pooling option is valid. 2) if model is from caffe, same option for pooling layer will be on.

piiswrong suggested changes Sep 30, 2016

View reviewed changes

rename kSame to kFull

5c31cbe

add some description

Darwin2011 changed the title ~~Add {valid, same} option for Pooling layer~~ Add {valid, full} option for Pooling layer Oct 1, 2016

Darwin2011 mentioned this pull request Oct 4, 2016

MKLdnn Integration Patch to improve issue #2986(call for cpu performance) #3438

Closed

piiswrong closed this Oct 5, 2016

sxjscience mentioned this pull request Oct 6, 2016

Pooling formula seems wrong to cause Googlenetv1 not working #3457

Closed

zhreshold added a commit to zhreshold/mxnet that referenced this pull request Oct 6, 2016

pre-merge pull request apache#3392

6ca3354

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add {valid, full} option for Pooling layer #3392

Add {valid, full} option for Pooling layer #3392

Darwin2011 commented Sep 28, 2016 •

edited

Loading

sxjscience commented Sep 28, 2016

Darwin2011 commented Sep 28, 2016

sxjscience commented Sep 28, 2016

Darwin2011 commented Sep 28, 2016

sxjscience commented Sep 28, 2016

sxjscience commented Sep 29, 2016

Darwin2011 commented Sep 29, 2016

sxjscience commented Sep 29, 2016

Darwin2011 commented Sep 29, 2016

piiswrong commented Sep 30, 2016 •

edited

Loading

piiswrong Sep 30, 2016

piiswrong Sep 30, 2016

sbodenstein commented Sep 30, 2016 •

edited

Loading

piiswrong commented Sep 30, 2016

Darwin2011 commented Oct 1, 2016

Darwin2011 commented Oct 1, 2016

sxjscience commented Oct 1, 2016

piiswrong commented Oct 3, 2016

Add {valid, full} option for Pooling layer #3392

Add {valid, full} option for Pooling layer #3392

Conversation

Darwin2011 commented Sep 28, 2016 • edited Loading

sxjscience commented Sep 28, 2016

Darwin2011 commented Sep 28, 2016

sxjscience commented Sep 28, 2016

Darwin2011 commented Sep 28, 2016

sxjscience commented Sep 28, 2016

sxjscience commented Sep 29, 2016

Darwin2011 commented Sep 29, 2016

sxjscience commented Sep 29, 2016

Darwin2011 commented Sep 29, 2016

piiswrong commented Sep 30, 2016 • edited Loading

piiswrong Sep 30, 2016

Choose a reason for hiding this comment

piiswrong Sep 30, 2016

Choose a reason for hiding this comment

sbodenstein commented Sep 30, 2016 • edited Loading

piiswrong commented Sep 30, 2016

Darwin2011 commented Oct 1, 2016

Darwin2011 commented Oct 1, 2016

sxjscience commented Oct 1, 2016

piiswrong commented Oct 3, 2016

Darwin2011 commented Sep 28, 2016 •

edited

Loading

piiswrong commented Sep 30, 2016 •

edited

Loading

sbodenstein commented Sep 30, 2016 •

edited

Loading