-
Notifications
You must be signed in to change notification settings - Fork 4
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
Decouple padding from keras frontend #143
Comments
Hi @TerrestrialEclipse,
I agree that this could be a great idea. Until now, I don't see any problems why this couldn't be implemented. |
Hi @pg020196, yes, that is what I would suggest. Up until now, padding has to be recomputed from the parameters in the intermediate representation. This implementation, however, only allows for symmetric padding. Still, asymmetric padding is possible, e.g. when pooling with a window size of 4x4 and padding "same" (in Keras), where padding width / height results in 3 in total, i.e. adding 1 at the beginning and 2 at the and of a column / row. As far as I understand, Pytorch on the other hand, explicitly requires specifying the number of padded elements as |
Hi @TerrestrialEclipse, Therefore, I would suggest to change the padding representation in the intermediate format in the before mentioned way. Additionally, instead of simply identifying the padding type the keras frontend should already translate the padding type (valid, same) into numeric values (top, bottom, left, right). Finally, the implementation in the .c-template file (or backend) must be adjusted to perform padding according to the given values. If I didn't forget anything, this would also allow the implementation of padding layers with asymmetric properties in future releases. I didn't have a look at the pytorch implementation but if you're right, the proposed solution would also work for a future pytorch frontend. |
Hi @pg020196 , as far as I understand, your current implementation tries to replicate the Keras behavior, but inherently, it only allows for symmetric padding: it computes a padding width and adds this exact amount both at the beginning and the end of a row / column. Keras, on the other hand, does allow for asymmetric padding implicitly, i.e., also without using an explicit padding layer. Maybe we should merge this issue with the second issue regarding padding (issue #144): That keras cuts off the filter kernel at the edges. Here is an example that showcases both issues of asymmetric padding width and cutting off kernels at the edges. I slightly adapted the model in the notebook
I use keras 2.5.0 and tensorflow 2.5.0-rc1. The total padding is
which yields
Here is a graphic representation of the filtering behavior in keras, where all
I think in the current implementation this is not supported. So far, this is also not supported in the C# backend, but I would add it once the intermediate format is adapted. |
In the intermediate representation padding (when used without explicit padding layer, e.g., as a parameter in pooling layers) isn't decoupled from the keras frontend.
Keras allows this padding to be asymmetric, while, e.g., pytorch only supports symmetric padding.
Hence, it would be good to have an explicit numerical representation of the padding sizes in the intermediate format. This would allow better decoupling of the intermediate format from the frontends.
The text was updated successfully, but these errors were encountered: