Skip to content
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

Padding type definition is swapped in the documentation. #196

Closed
cesarsalgado opened this issue Nov 13, 2015 · 5 comments
Closed

Padding type definition is swapped in the documentation. #196

cesarsalgado opened this issue Nov 13, 2015 · 5 comments
Labels
type:docs-bug Document issues

Comments

@cesarsalgado
Copy link
Contributor

In http://tensorflow.org/api_docs/python/nn.md#pooling and http://tensorflow.org/api_docs/python/nn.md#convolution

The definition of the padding types should be swapped.

Now it is like this:

padding = 'SAME': Round down (only full size windows are considered).
padding = 'VALID': Round up (partial windows are included).

But it should be like this:

padding = 'SAME': Round up (partial windows are included).
padding = 'VALID': Round down (only full size windows are considered).

I also think it should be made clear that the padding is already included in the shape(value) of the formula "shape(output) = (shape(value) - ksize + 1) / strides".

Or maybe the formula should be changed to the following simpler one (taken from http://cs231n.github.io/convolutional-networks/#conv):

shape(output) = ((shape(value)+2*pad - ksize) / strides) + 1

where pad = 0 if padding='VALID' or pad = floor(ksize/2) if padding='SAME'.

@teamdandelion teamdandelion added the type:docs-bug Document issues label Nov 13, 2015
@Yangqing
Copy link
Contributor

Thanks - the padding documentation definitely needs modification. The padding scheme is not as simple as you mentioned though - it follows some legacy reason which involved some non-trivial padding computations. I'll update the documentation to add the details.

For the time being, if you are interested in the the exact math, it is described here:

https://github.com/Yangqing/caffe2/blob/master/caffe2/proto/caffe2_legacy.proto#L8

@Yangqing
Copy link
Contributor

Thanks again for reporting this. We have fixed this in our internal codebase and will be pushing it soon. Closing this for now.

@NianzuMa
Copy link

NianzuMa commented Jan 2, 2016

Propose to enrich the document of convolution. It is still quite confusing.
http://tensorflow.org/api_docs/python/nn.md#convolution

I think what cesarsalgado mentioned is correct.

@jiankang1991
Copy link

Could anyone help me to calculate each step output of cifar10 example of tensorflow?
The input image size is 24*24. But in max pooling step, the kernel size is 3 and stride is 2, so (24-3)/2+1 is not integer. What is wrong here? I am new in deep learning. Thank you for helping

@sesse
Copy link

sesse commented May 31, 2016

Hi,

Beginner questions are best suited for Stack Overflow, not random semi-related bugs. Please post your question there instead.

lissyx pushed a commit to lissyx/tensorflow that referenced this issue Feb 16, 2018
cjolivier01 pushed a commit to Cerebras/tensorflow that referenced this issue Dec 6, 2019
…e-python3

Update tensorflow-quickstart.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:docs-bug Document issues
Projects
None yet
Development

No branches or pull requests

6 participants