-
Notifications
You must be signed in to change notification settings - Fork 31
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
Added valid and fixed padding. #43
Conversation
Thanks for the PR! I've described some suggested updates here. In the meantime, I just wanted to understand a bit more about how you did the validation in Tensorflow and Pytorch --- would you share what you did, and perhaps an example set of commands corresponding to one of the unit tests you added?) It would be really helpful for other readers to have evidence that we're following the correct convention (I previously had an off-by-one issue when dealing with even-sized kernels). |
My original scripts were not exactly elegant, so here are two improved scripts to generate the convolution output.
And here's PyTorch's one. Note: PyTorch does not support "same" padding, so I didn't test it.
|
Thank you for sharing the scripts (and spending the time to clean them up!) The comments are particularly helpful, and I'll use definitely be using it in the future if I'm trying to provide new functionality that matches the behavior on Pytorch. I might actually merge these scripts in to the repo as part of a later PR so that everything is fully repeatable. |
Suggested updates to PR vtjeng#43
Glad to be of help! |
+ Make identification of repo root more robust. + Improve readability of multiline `julia` command.
Using #16 as a starting point, I added support for both valid and fixed padding.
Fixed padding can be passed as
Int64
(same padding for all edges), a tuple of size 2 (y_padding, x_padding) or a tuple of size 4 (top, bottom, left, right).The implementation follows Tensorflow's conventions, but just to be sure I checked that it matches PyTorch's implementation too.
This is my first time working on MIPVerify, so I might have been a bit overzealous with the unit tests. If that's the case, I'll remove some of them.
Edit: passing -> padding