Add circular padding to flax.linen.Conv and flax.linen.ConvTranspose #1661
Conversation
|
Should ConvTranspose have a circular padding option as well? |
|
It does make sense to me. I was just wondering, in your mini example, what
if the filter size is 4 (or any even number)?
…On Fri, 5 Nov 2021, 04:29 jheek, ***@***.***> wrote:
Should ConvTranspose have a circular padding option as well?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1661 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHQJA73PKWCUTVMERS6E5QLUKOPYJANCNFSM5HNK7I3A>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
*Aviso legal:* El contenido de este mensaje y los archivos adjuntos son
confidenciales y de uso exclusivo de la Universidad Nacional de Colombia.
Se encuentran dirigidos sólo para el uso del destinatario al cual van
enviados. La reproducción, lectura y/o copia se encuentran prohibidas a
cualquier persona diferente a este y puede ser ilegal. Si usted lo ha
recibido por error, infórmenos y elimínelo de su correo. Los Datos
Personales serán tratados conforme a la Ley 1581 de 2012 y a nuestra
Política de Datos Personales que podrá consultar en la página web
www.unal.edu.co <http://www.unal.edu.co/>.* *Las opiniones, informaciones,
conclusiones y cualquier otro tipo de dato contenido en este correo
electrónico, no relacionados con la actividad de la Universidad Nacional de
Colombia, se entenderá como personales y de ninguna manera son avaladas por
la Universidad.
|
@jheek That would make sense, at least that seems to be implemented in Would you recommend adding it to this PR or first have this one reviewed and then create a follow-up? |
I think it makes sense to add it to both conv options in this PR. This way we make sure that both conv layers are consistent at that they are indeed transposable |
@VolodyaCO Indeed, if filter size is even, one can't perfectly centre the filter around each element of the input, so there needs to be convention - either shift it by one element forwards or backwards. Current implementation for So, for example, for input (1, 2, 3, 4, 5), kernel (1, 1, 1, 1), stride 3, and Again, as I said, choosing to shift the kernel one element forwards or backwards for even-sized kernels is purely a convention, but I just made sure it's consistent between |
@jheek Ok, perfect, let me add it then |
|
Looks good to me! Thanks a lot
…On Fri, 5 Nov 2021 at 08:48, Grisha Oryol ***@***.***> wrote:
Would you recommend adding it to this PR or first have this one reviewed
and then create a follow-up?
I think it makes sense to add it to both conv options in this PR. This way
we make sure that both conv layers are consistent at that they are indeed
transposable
@jheek <https://github.com/jheek> Ok, perfect, let me add it then
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1661 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHQJA756NSCHOPT62FXFYJ3UKPOB7ANCNFSM5HNK7I3A>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
Best wishes,
Vladimir Vargas-Calderón
<https://www.researchgate.net/profile/Vladimir_Vargas-Calderon>
PhD Physics Student @ Universidad Nacional de Colombia
--
*Aviso legal:* El contenido de este mensaje y los archivos adjuntos son
confidenciales y de uso exclusivo de la Universidad Nacional de Colombia.
Se encuentran dirigidos sólo para el uso del destinatario al cual van
enviados. La reproducción, lectura y/o copia se encuentran prohibidas a
cualquier persona diferente a este y puede ser ilegal. Si usted lo ha
recibido por error, infórmenos y elimínelo de su correo. Los Datos
Personales serán tratados conforme a la Ley 1581 de 2012 y a nuestra
Política de Datos Personales que podrá consultar en la página web
www.unal.edu.co <http://www.unal.edu.co/>.* *Las opiniones, informaciones,
conclusiones y cualquier otro tipo de dato contenido en este correo
electrónico, no relacionados con la actividad de la Universidad Nacional de
Colombia, se entenderá como personales y de ninguna manera son avaladas por
la Universidad.
|
sgrigory
changed the title
|
@jheek I've added circular padding to |
| @@ -204,6 +204,174 @@ def test_group_conv(self): | |||
| self.assertEqual(initial_params['params']['kernel'].shape, (3, 2, 4)) | |||
| np.testing.assert_allclose(y, np.full((1, 6, 4), 7.)) | |||
|
|
|||
| @parameterized.product( | |||
| rhs_dilation=self.kernel_dilation, | ||
| precision=self.precision) | ||
|
|
||
| if self.padding == "CIRCULAR": |
There was a problem hiding this comment.
this is pretty complicated. Is there a reference for this?
There was a problem hiding this comment.
@jheek I didn't use any reference (except for verifying the results against neural_tangents.stax.ConvTranspose), but let me explain the logic here:
After ConvTranspose has been calculated with "VALID" padding into variable y in this line, we need to
- identify the size of the final output along each spatial dimension (let's call it "period")
- pad each dimension to a certain integer number of periods
- wrap the array periodically around each spatial dimension
Step by step:
-
Size of the final output is stride * input size
-
For padding, we need to understand how much to pad from left/right for each spatial dimension. Padding should satisfy three criteria:
- total size after padding should be an integer number of periods
- padding should be the symmetric (same from the right and from the left)
- the element corresponding to beginning of the original input data inside the padded array should be located at integer number of periods (otherwise we'll get correct answer, but circularly shifted)
The above is satisfied if we compute the padding like I did in the code:
- Compute the difference between the size of
yand the desired size of the final output - Compute complement of this difference to even number of periods
- Divide the complement into two for left and right padding
- After the padding is done, we can wrap the array around each spatial dimension. I did this separately for each dimension, reshaping the array into (..., -1, period, ...) and summing over the corresponding axis. Let me know if there is a simpler way to do it - I haven't found one (see e.g. https://stackoverflow.com/questions/42297115/numpy-split-cube-into-cubes)
There is a subtlety with even-sized kernels: the choice to write (size_diff + 1) // 2, size_diff // 2) (and not (size_diff // 2, (size_diff + 1) // 2)) here reflects the alignment convention I talk about in a comment above
What's the best way to proceed, should I add some of this description to inline comments?
There was a problem hiding this comment.
Yes I think it makes sense to add (a summary of) in inline comments
There was a problem hiding this comment.
@jheek Sure, I've expanded the comments in ConvTranspose - please have a look
Codecov Report
@@ Coverage Diff @@
## main #1661 +/- ##
==========================================
+ Coverage 83.10% 83.17% +0.06%
==========================================
Files 69 69
Lines 5836 5853 +17
==========================================
+ Hits 4850 4868 +18
+ Misses 986 985 -1
Continue to review full report at Codecov.
|
sgrigory
force-pushed
the
971-add-circular-padding
branch
from
1565286 to
95ea665
Compare
What does this PR do?
This PR implements a
CIRCULARpadding option forflax.linen.Convandflax.linen.ConvTranspose, in addition to existingVALIDandSAME. This allows creating convolutional and transposed convolutional layers with periodic boundary conditions. Code added toflax.linen.Convis based on the snippet from #903 (comment). Also tests are added for the new padding optionFixes #971, #903
Checklist
checks if that's the case).
documentation guidelines.
(No quality testing = no merge!)
Additional note
Please note that this implementation differs slightly from suggested in #971 (comment) in the sense that the circular padding is applied symmetrically from both sides of the input, as in #903 (comment).
For example, on 1D data [1, 2, 3, 4, 5] with kernel size 3 and stride 3 there will be 2 filter operations: one at [5, 1, 2] and the other at [3, 4, 5] - instead of [1, 2, 3] and [4, 5, 1], as suggested in #971 (comment). I feel that this is better aligned with how other padding options -
VALIDandSAME- work.@VolodyaCO, please let me know if that makes sense for your use case