add adaptive pool

[dnabanita7]

I have added AdaptiveMaxPool and AdaptiveMeanPool so that we can do a similar PyTorch implementation. cc @darsnack

PR

• Tests are added
• Entry in NEWS.md
• Documentation, if applicable
• Final review from @MikeInnes or @dhairyagandhi96 (for API changes).

Flux issue linking

Flux#1224

MLH issue linking

0.3.x-projects#26

[darsnack]

@dhairyagandhi96 do you know why no CI is being performed for this PR?

[CarloLucibello]

weird

[darsnack]

The PR contains the CI YAML files as far as I can tell...

Could bors be used to trigger?

[dnabanita7]

Shall I use GitHub Actions to add the CI workflow? I will create another PR for it.

[darsnack]

@dnabanita7 can you try forcing an empty commit with no changes?

The CI should already be set up. It works for all the other commits..

[dnabanita7]

It works now.

[darsnack]

Yeah thanks @CarloLucibello for restarting it!

[darsnack]

Since you need to make another commit anyways, can you change the naming from in_size -> insize (same for other variables with underscores).

[darsnack]

Since you are removing the comments, I would get rid of the blank lines in between. Personally I would only have a blank line before the return statement.

[dnabanita7]

PoolDims(::Tuple{Vararg{T,M}} where T, or PoolDims(::Union{Int64, Tuple{Vararg{Int64,L}}}; stride, padding, dilation) where {M, L} shall I change the dispatch in struct according to this?

[dnabanita7]

Since you are removing the comments, I would get rid of the blank lines in between. Personally I would only have a blank line before the return statement.

The spaces between functions too? Well other functions don't have that in that file.

[CarloLucibello]

Flux uses 2 whitespaces indent (unfortunately in my opinion), so you should correct all indents, except for docstrings (where you should leave 4)

[darsnack]

The spaces between functions too? Well other functions don't have that in that file.

No that comment is out of date. Please ignore.

[darsnack]

PoolDims(::Tuple{Vararg{T,M}} where T, or PoolDims(::Union{Int64, Tuple{Vararg{Int64,L}}}; stride, padding, dilation) where {M, L} shall I change the dispatch in struct according to this?

What exactly do you mean?

[dnabanita7]

The error shown is method mismatch. Shall I change the constructor in struct to the above mentioned?

[darsnack]

The error is from a constructor in NNlib.jl. You aren't dispatching to https://github.com/FluxML/NNlib.jl/blob/aa4bb07f35322498c9fabd391ab1ecd73d09afd4/src/dim_helpers/PoolDims.jl#L24 because the method signature says k needs to be a tuple of ints (or a single int), and you are passing a tuple of floats.

[dnabanita7]

I guess its good to get a final review from @DhairyaLGandhi or @MikeInnes

[avik-pal]

@dnabanita7 An entry in the NEWS.md would be needed for this.

[dnabanita7]

cc @DhairyaLGandhi

[DhairyaLGandhi]

Can we also add the tests to the GPU tests. I know maxpool is already in the test suite, but good for making sure we are doing layer regression tests consistently.

[DhairyaLGandhi]

There's a few other places that would need the \div treatment. Thanks for looking into it!

[CarloLucibello]

thanks for this contribution

bors r+

[bors]

Build succeeded:

• ci/gitlab/gitlab.com