Flux.jl: Support for asymmetric padding

[ayush1999]

While loading the GoogleNet ONNX model, I came across a padding of the type [0,0,2,2]. Due to its asymmetric nature, it couldn't be implemented in Flux.

As @tejank10 and I were talking, we think its time Flux starts supporting asymmetric padding. It's strange that nobody ever felt a need for this before.

[jekbradbury]

Some background (see https://github.com/onnx/onnx/issues/401#issuecomment-356281852):

What happens is that nobody ever _wants_ to use asymmetric padding, but sometimes it's impossible to have perfectly symmetric padding (I believe if the input and output sizes aren't either both even or both odd). And different frameworks have historically made different assumptions about _which asymmetry_ to choose when they're asked to pad symmetrically but can't make it perfectly symmetric. So ONNX has to represent these differences explicitly, which means we probably want to figure out what choice Flux/NNlib is implicitly making and also provide a means to bypass that choice and use explicitly provided asymmetries...

[jekbradbury]

More digging: I think this is where the asymmetry comes from https://github.com/pytorch/pytorch/blob/master/caffe2/onnx/onnx_exporter.cc#L491 and this is what Caffe2 itself does when it sees such an asymmetry https://github.com/pytorch/pytorch/blob/master/caffe2/operators/pool_op_cudnn.cu#L225. If this really does originate from Caffe "legacy padding behavior" (https://github.com/pytorch/pytorch/blob/master/caffe2/proto/caffe2_legacy.proto#L23) then we may still have problems related to the different output size? But if it originates from DistBelief legacy behavior (the top two options in that proto) then replacing asymmetric padding with the nearest symmetric version is probably OK.

[MikeInnes]

Worth figuring out if we can do "nearest symmetric" here, or otherwise convert back to symmetric padding. @ayush1999 perhaps do some digging and figure out what we're doing implicitly here?

If we go for this it'll be quite an invasive change over the convolution stack, though I'm sure we can keep the current API everywhere by default.

[ayush1999]

Asymmetric padding generally arises when dealing with models made using Caffe. Currently, we specify padding in Flux like (a,b), which is actually expanded to (a,b,a,b). However, padding in Caffe is written as (a,b,c,d). As you mentioned, we can convert asymmetric padding into symmetric padding, as I have been doing in ONNX.jl (here). However, there are a few shortcomings:

1. It affects the performance of the model, which is fairly obvious.

2. In many cases, asymmetric padding cannot be converted to symmetric padding, without changing the output shape. For example, it isn't possible to convert (0,0,1,1) or (0,0,3,3) to symmetric padding.

[ianshmean]

It would be good to re-start this discussion (after it being brought up in the machine-learning slack channel). In addition to the above mentioned GoogleNet issue, this prevents the tony_yolov2 ONNX models from being usable in flux, resulting in a 12×12×125×1 output rather than the expected 13×13×125×1

[lazarusA]

How is this going?, still in progress.... ??

[bhvieira]

@MikeInnes this works already, right?

[ianshmean]

According to https://github.com/FluxML/NNlib.jl/pull/84#issuecomment-481126094 it is.

I believe ONNX still needs some work, but I haven't re-tested the yolov2 ONNX models I mentioned above

[bhvieira]

@dhairyagandhi96 or @CarloLucibello I think this issue can be safely closed, asymmetric padding has been merged a long time ago

[DhairyaLGandhi]

Thanks!

[AStupidBear]

But assymetric padding does not work for CUDNN.

using Flux
x = randn(100, 100, 7, 100) |> gpu;
model = Conv((10,10), 7 => 3, pad = (9, 0, 9, 0)) |>gpu;
model(x)
┌ Warning: CuDNN does not support asymmetric padding; defaulting to symmetric choice
└ @ CuArrays.CUDNN ~/shared/.julia/packages/CuArrays/e8PLr/src/dnn/conv.jl:49
ERROR: CUDNNError: CUDNN_STATUS_BAD_PARAM (code 3)

[AStupidBear]

Some background (see onnx/onnx#401 (comment)):

What happens is that nobody ever _wants_ to use asymmetric padding, but sometimes it's impossible to have perfectly symmetric padding (I believe if the input and output sizes aren't either both even or both odd). And different frameworks have historically made different assumptions about _which asymmetry_ to choose when they're asked to pad symmetrically but can't make it perfectly symmetric. So ONNX has to represent these differences explicitly, which means we probably want to figure out what choice Flux/NNlib is implicitly making and also provide a means to bypass that choice and use explicitly provided asymmetries...

@jekbradbury Assymetric padding is also used in causal Conv1D. See keras-tcn.

[bhvieira]

The issue is still Open over there as well: https://github.com/JuliaGPU/CuArrays.jl/issues/356, didn't see it before.
Some temporary solutions are proposed there.