-
Notifications
You must be signed in to change notification settings - Fork 18.6k
Padding for Max Pooling #473
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
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 :) |
|
I agree with dividing by the actual number of elements in the pooling 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
|
|
Good luck with NIPS :) |
|
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). |
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.
[Adapted from a commit by @jeffdonahue by @shelhamer.]
|
@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. |
|
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. |
|
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). |
Padding for Max Pooling
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.