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

ConvTranspose errors with symmetric non-constant pad #2424

Closed
piever opened this issue Apr 17, 2024 · 0 comments · Fixed by #2463
Closed

ConvTranspose errors with symmetric non-constant pad #2424

piever opened this issue Apr 17, 2024 · 0 comments · Fixed by #2463

Comments

@piever
Copy link
Contributor

piever commented Apr 17, 2024

Description

The ConvTranspose layer errors if given a symmetric non-constant pad (that is, length(pad) == ndims(w) - 2), whereas pad::Int (constant pad) and length(pad) == 2 * (ndims(w) - 2) (non-symmetric non-constant pad) work fine.

MWE:

using Flux

c = ConvTranspose((3, 3), 1 => 1, pad = (1, 0))

x = randn(Float32, 5, 5, 1, 16)

c(x)

errors with

ERROR: MethodError: no method matching DenseConvDims(::Tuple{Int64, Int64, Int64}, ::NTuple{4, Int64}; stride::Tuple{Int64, Int64}, padding::Tuple{Int64, Int64}, dilation::Tuple{Int64, Int64}, groups::Int64)

Closest candidates are:
  DenseConvDims(::Tuple{Vararg{Int64, N}}, ::Tuple{Vararg{Int64, K}}, ::Int64, ::Int64, ::Int64, ::Tuple{Vararg{Int64, S}}, ::Tuple{Vararg{Int64, P}}, ::Tuple{Vararg{Int64, D}}, ::Bool) where {N, K, S, P, D} got unsupported keyword arguments "stride", "padding", "dilation", "groups"
   @ NNlib C:\Users\pietro\.julia\packages\NNlib\O0zGY\src\dim_helpers\DenseConvDims.jl:7
  DenseConvDims(::Tuple{Vararg{T, M}} where T, ::Tuple{Vararg{T, M}} where T; stride, padding, dilation, groups, flipkernel) where M
   @ NNlib C:\Users\pietro\.julia\packages\NNlib\O0zGY\src\dim_helpers\DenseConvDims.jl:20

Stacktrace:
 [1] conv_transpose_dims(c::ConvTranspose{2, 2, typeof(identity), Array{Float32, 4}, Vector{Float32}}, x::Array{Float32, 4})
   @ Flux C:\Users\pietro\.julia\packages\Flux\Wz6D4\src\layers\conv.jl:323
 [2] (::ConvTranspose{2, 2, typeof(identity), Array{Float32, 4}, Vector{Float32}})(x::Array{Float32, 4})
   @ Flux C:\Users\pietro\.julia\packages\Flux\Wz6D4\src\layers\conv.jl:336
 [3] top-level scope
   @ REPL[6]:1

Environment

The above reproducer works in Julia 1.10.2 on a fresh environment

(jl_tMb72B) pkg> st
Status `C:\Users\pietro\AppData\Local\Temp\jl_tMb72B\Project.toml`
  [587475ba] Flux v0.14.15

Suggestion

I suspect the problem is in combined_pad (here and here). It assumes that c.pad has length 2 * (ndims(c.weight) - 2), which is not the case.

paulnovo added a commit to paulnovo/Flux.jl that referenced this issue Jun 22, 2024
The ConvTranspose was not able to handle symmetric non-constant
padding (ie, `pad=(1, 0)` for 2D ConvTranspose). Constant padding (ie
`pad=1` for 2D ConvTranspose) and assymetric non-constant padding (ie,
`pad=(1, 0, 2, 3)`) worked correctly. This commit fixes symmetric
non-constant padding and adds unit tests to ensure it produces the same
output size as an equivalent fully expanded padding.

Fixes FluxML#2424
paulnovo added a commit to paulnovo/Flux.jl that referenced this issue Jun 30, 2024
The ConvTranspose was not able to handle symmetric non-constant
padding (ie, `pad=(1, 0)` for 2D ConvTranspose). Constant padding (ie
`pad=1` for 2D ConvTranspose) and assymetric non-constant padding (ie,
`pad=(1, 0, 2, 3)`) worked correctly. This commit fixes symmetric
non-constant padding and adds unit tests to ensure it produces the same
output size as an equivalent fully expanded padding.

Fixes FluxML#2424
paulnovo added a commit to paulnovo/Flux.jl that referenced this issue Jul 28, 2024
The ConvTranspose was not able to handle symmetric non-constant
padding (ie, `pad=(1, 0)` for 2D ConvTranspose). Constant padding (ie
`pad=1` for 2D ConvTranspose) and assymetric non-constant padding (ie,
`pad=(1, 0, 2, 3)`) worked correctly. This commit fixes symmetric
non-constant padding and adds unit tests to ensure it produces the same
output size as an equivalent fully expanded padding.

Fixes FluxML#2424
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants