Skip to content

Conversation

@shelhamer
Copy link
Member

Max pooling pads by -inf if the padding parameter is set.

Padding for pooling, like padding for convolution, can preserve the
dimensions of the bottom at the top. By setting the padding to
floor(kernel_size / 2) the top output is the "same" instead of the
"valid" part of the bottom input.

Note: I should add a test for max pooling with padding. Included Jeff's test and other polish in 2239188.

@Yangqing
Copy link
Member

Yangqing commented Jun 6, 2014

A minor suggestion: maybe for average pooling we should divide by the number of elements in the pooling region rather than padding 0 values and divide by the general kernel size? This would only affect a few outputs on the boundary, so both solutions work for me :)

@shelhamer
Copy link
Member Author

I agree with dividing by the actual number of elements in the pooling
region. That's what I do in my private stash of pooling and resampling
layers to avoid boundary artifacts.

I'm happy to follow-up with this average pooling fix in a post-NIPS PR.

On Fri, Jun 6, 2014 at 9:59 AM, Yangqing Jia notifications@github.com
wrote:

A minor suggestion: maybe for average pooling we should divide by the
number of elements in the pooling region rather than padding 0 values and
divide by the general kernel size? This would only affect a few outputs on
the boundary, so both solutions work for me :)


Reply to this email directly or view it on GitHub
#473 (comment).

@Yangqing
Copy link
Member

Yangqing commented Jun 6, 2014

Good luck with NIPS :)

@jeffdonahue
Copy link
Contributor

There is a slight oddity with padding in the PoolingLayer -- namely it already does some of its own "padding" by default, as it will always place enough "pools" on the image to cover the entire thing starting from the top-left, even if that requires placing regions on the lower & right borders that extend past the image (padding).

This is in contrast with the ConvolutionLayer, which only considers "valid" convolutions, probably because @Yangqing designed it with the intention of using explicitly padding if one wants to ensure coverage of the entire image, whereas the PoolingLayer has this built-in implicit padding (at the bottom and right of the image) for some convenience at the cost of some control. Not necessarily with this PR, but eventually it might be nice to have a proto switch that sets the mode of the pooling (e.g. COVER [default] vs VALID).

shelhamer and others added 2 commits June 7, 2014 22:55
Max pooling pads by -inf if the padding parameter is set.

Padding for pooling, like padding for convolution, can preserve the
dimensions of the bottom at the top. By setting the padding to
floor(kernel_size / 2) the top output is the "same" instead of the
"valid" part of the bottom input.
@shelhamer
Copy link
Member Author

@jeffdonahue I included your test, the padding check for the last pooling and other polish. If you like, check that I properly cherry-picked and merge.

@shelhamer
Copy link
Member Author

Regarding COVER vs. VALID pooling, should COVER actually be an option? SAME and VALID are reasonable and symmetric, but the COVER pool from the beginning {top,left} and off the end {bottom,right} seems incidental. Perhaps a switch is good for compatibility all the same since COVER is how we've pooled.

jeffdonahue added a commit that referenced this pull request Jun 8, 2014
@jeffdonahue jeffdonahue merged commit adb2206 into BVLC:dev Jun 8, 2014
@jeffdonahue
Copy link
Contributor

Looks good, thanks Evan! I only thought COVER should be an option for compatibility as well -- "SAME" would probably be a good default assuming it means to pad such that the pooled output size is the same as the input size, though I'm not sure how we'd want to handle stride>1 then (maybe it's ok for SAME to only actually mean SAME in the stride==1 case).

@shelhamer shelhamer deleted the pad-max-pooling branch June 25, 2014 21:54
@shelhamer shelhamer mentioned this pull request Aug 7, 2014
mitmul pushed a commit to mitmul/caffe that referenced this pull request Sep 30, 2014
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.

3 participants