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

Add output padding for ConvTranspose #2462

Merged
merged 1 commit into from
Jun 30, 2024
Merged

Conversation

guiyrt
Copy link
Contributor

@guiyrt guiyrt commented Jun 22, 2024

This PR provides the ability to increase ConvTranspose's output shape when stride > 1, implemented in the same way as in PyTorch output_padding (see ConvTranspose3d and NaiveConvolutionTranspose3d.cpp).

The purpose of this is to conserve Conv/ConvTranspose inversability. For stride = N, N>1, there are N possibilities per dimension that result in the same Conv output (see what-output-padding-does-in-nn-convtranspose2d). The added argument outpad controls how much the output shape is increased to match the strided Conv counterpart. Can be provided as an integer to equally increase all dimensions, or as a tuple for individual dimensional control).

outpad is invalid if any outpad .>= stride. Currently there is no verification of this when constructing ConvTranspose, it only breaks when called and throws DimensionMismatch. A similar thing happens in PyTorch, it allows constructing ConvTransposeNd with invalid output_paddding, but then throws RuntimeError before doing the deconvolution. As the verification can be done in the constructor, it makes sense to handle it there, but let me know what you think.

I tried to follow the variable naming style, but it can also be changed if you see a better fitting alternative. The same applies for other things such as argument order.

PR Checklist

  • Tests are added
  • Entry in NEWS.md
  • Documentation, if applicable

@CarloLucibello CarloLucibello merged commit 36abc73 into FluxML:master Jun 30, 2024
5 of 8 checks passed
@CarloLucibello
Copy link
Member

thanks!

@guiyrt guiyrt deleted the outpad branch June 30, 2024 11:25
@paulnovo
Copy link
Contributor

Hi @CarloLucibello and @guiyrt, I was wondering if the output padding should also be applied in the AMDGPU implementation of conv_transpose_dims?

I am new to Flux, but I noticed this when rebasing my changes to ConvTranspose, so I thought I should ask.

@pxl-th
Copy link
Member

pxl-th commented Jul 11, 2024

@paulnovo yes, currently ConvTranspose is broken for AMDGPU because of this:
https://buildkite.com/julialang/flux-dot-jl/builds/4674#01903fca-3e24-4906-81a2-728846992074/361-742

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants