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

Incorrect ZeroPadding before MaxPool2D in keras' resnet #48105

Closed
dlopr opened this issue Mar 26, 2021 · 6 comments
Closed

Incorrect ZeroPadding before MaxPool2D in keras' resnet #48105

dlopr opened this issue Mar 26, 2021 · 6 comments
Assignees
Labels
comp:keras Keras related issues stat:awaiting tensorflower Status - Awaiting response from tensorflower type:docs-bug Document issues

Comments

@dlopr
Copy link

dlopr commented Mar 26, 2021

Hi,

I've noticed that the implementation of resnet networks in keras introduces a ZeroPadding layer before the initial MaxPool2D 3x3 stride 2:

x = layers.ZeroPadding2D(padding=((1, 1), (1, 1)), name='pool1_pad')(x)

I don't think this is correct. Zero is not a neutral element for a MaxPool2D operation. The input values at the edges could be negative.

I believe the intention of that zero padding layer is what would be correctly represented as SAME padding for the MaxPool2D directly. Note that the padding layer is also adding padding elements to the right and bottom edges of the input that the 3x3 stride 2 MaxPool2D operation won't ever use if the input is even size as I believe it commonly is.

@dlopr dlopr changed the title ZeroPadding before a MaxPool2D doesn't look correct ZeroPadding before MaxPool2D in keras' resnet doesn't look correct Mar 26, 2021
@dlopr dlopr changed the title ZeroPadding before MaxPool2D in keras' resnet doesn't look correct Incorrect ZeroPadding before MaxPool2D in keras' resnet Mar 26, 2021
@Saduf2019 Saduf2019 self-assigned this Mar 26, 2021
@Saduf2019 Saduf2019 added comp:keras Keras related issues type:docs-bug Document issues labels Mar 26, 2021
@jvishnuvardhan jvishnuvardhan added the stat:awaiting tensorflower Status - Awaiting response from tensorflower label Mar 26, 2021
@xonx4l
Copy link

xonx4l commented Aug 15, 2023

Hey! here's a solution I think that can be implemented .

@xonx4l
Copy link

xonx4l commented Aug 15, 2023

Here's a pr in eras repo as i am unable to find keras/application on the tensorflow repo .

-:Update resnet.py #1

@xonx4l
Copy link

xonx4l commented Aug 15, 2023

In the PR-I've changed the pool1_pad layer's padding to (1, 0) for the right and bottom edges. This will ensure that the padding is added only on the top and left sides, which compensates for the even-sized input.
I've changed the MaxPooling2D layer's padding to 'valid', which is equivalent to "SAME" padding in this context because of the additional padding added by pool1_pad.

@dlopr
Copy link
Author

dlopr commented Aug 15, 2023

I can't find the pr you're referring to.

In any case, if I understood you correctly, removing the right and bottom padding would only avoid wasteful addition of elements that won't be used.

The main issue was that a value of zero is not correct for padding the input of a maxpool layer with signed type. It should be padded with the minimum representable value of the type or just use same padding and avoid the pad layer if supported.

@MarkDaoust
Copy link
Member

Hi, the source for the keras code is now github.com/keras-team/keras and soon github.com/keras-team/keras-core.

Actually keras.applications is in maintenance mode. users should prefer the implementations in github.com/keras-team/keras-cv.

Either way, if this issue still exists, please re-raise it on the Keras repo(s).

@google-ml-butler
Copy link

Are you satisfied with the resolution of your issue?
Yes
No

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:keras Keras related issues stat:awaiting tensorflower Status - Awaiting response from tensorflower type:docs-bug Document issues
Projects
None yet
Development

No branches or pull requests

5 participants