Ability to pass groupcount parameter for depthwise and groupwise convolutions

[arhik]

With the change in #521, We need an api to set groupcount. I am proposing this as a draft and I am planning to make corresponding changes in NNlib. This will break existing NNlib interface since groupcount is an extra argument to ConvDesc.

Please feel free to comment on this once I have one of these check boxes complete.

• Add groupcount to NNlib ConvDims as a common Descriptor and have seperate constructs in Fluxml.

[arhik]

I need help with these shims.

[MikeInnes]

Would be great to have a look from @staticfloat for NNlib compatibility.

[arhik]

Would be great to have a look from @staticfloat for NNlib compatibility.

@staticfloat NNlib related changes are in FluxML/NNlib.jl#146 (comment)

These are complete set of changes to test:
Pkg.add(PackageSpec(url="https://github.com/arhik/Zygote.jl.git#groupwise"))
Pkg.add(PackageSpec(url="https://github.com/arhik/CuArrays.jl.git#groupcount"))
Pkg.add(PackageSpec(url="https://github.com/arhik/NNlib.jl.git#common_convdims"))
Pkg.add(PackageSpec(url="https://github.com/arhik/Flux.jl.git#groupwiseconv"))

I have few test cases ready. I will push them whenever I can.

Please observe the branche names after hashes.

[denizyuret]

@maleadt any progress on this?

[maleadt]

No, I'm not working on any of this.

[denizyuret]

@DhairyaLGandhi, is this ball in your court? We need group convolutions.

[k8lion]

Any progress? As others have said on the related issue thread on Flux.jl, my research also relies on depthwise convolutions on GPU.

[denizyuret]

I implemented this in the latest master.

…

On Sat, Feb 6, 2021 at 2:55 PM k8lion ***@***.***> wrote: Any progress? As others have said on the related issue threads, my research also relies on depthwise convolutions on GPU. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#523 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN43JU2NRX5TQPKFNG55R3S5UUZLANCNFSM4JTFB67A> .

[k8lion]

Great, so now looks NNlib needs to be modified in order for Flux.DepthwiseConv to be correctly evaluated on GPU?

[denizyuret]

Correct. Or use Knet.

…

On Sun, Feb 7, 2021 at 4:41 PM k8lion ***@***.***> wrote: Great, so now looks like Flux and NNlib need to be modified to correctly use DepthwiseConv on GPU (or add the groups parameter to Conv)? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#523 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN43JUUEP3EO5NOFMY2KYDS52J7LANCNFSM4JTFB67A> .

[DhairyaLGandhi]

Now that's on cuda master, we should push the same change on NNlib as well.

[arhik]

Sorry I didnt have GPU hardware to work and test any of this after pull request. You can follow the changes I made in NNlib, Zygote and Flux if you agree with FluxML/NNlib.jl#146 (comment) . I am not upto date with Zygote, NNlib and Flux development to judge how easy to translate my changes ...

Edit:
Just checked the diffs. They are easy to translate. Also just to want you : I remember I didn't focus on CPU API. I was only focussed on GPU.

Any one interested in this issue can also leverage my mobilenet implementation here. https://github.com/arhik/Flux_mobilenet.git

Cool that this is in limelight. Cant wait to see groupwise convolutions in Flux.