-
-
Notifications
You must be signed in to change notification settings - Fork 83
WIP: Add wrapper functions for setting/getting convolution group count #388
Conversation
Reposting from the original thread
You can pass the group count as a keyword argument maybe, but even I am not certain if this is the right way or we should modify NNlib cdims to incorporate it internally. @dhairyagandhi96 thoughts on the best way to do this? |
As avik-pal pointed out, there could be a few potential points of integration, but I don't have the overview to see which are feasible and sensible. Would be great to get some feedback from the library maintainers and I am happy to try draft up an implementation based on it. Supporting group convolution is highly relevant to efficiently implement several modern NN architectures. |
I believe the correct way would be to capture it in the cdims object. Pinging @staticfloat for his thoughts |
Yes; we can make the This will require us to write an implementation of grouped convolution so that we can test all our implementations against eachother, but that shouldn't be too hard. |
True and along the same line of argumentation a regular convolution is simply a special case with groupcount==1 (the default value)? |
Yes, that's true. So really, we could do away with |
I like the sound of that, Avik had a similar thing in mind as well |
Concerning this PR: the API wrapper is already in CuArrays, so we only need an API: #523 |
Based on a discussion over at Flux ( FluxML/Flux.jl#459 and FluxML/Flux.jl#330 ) @avik-pal proposed the inclusion of new functions in libcudnn.jl, namely to set the group count for a group convolution.
Further changes to other functions to actually enable group convolutions (and by that depthwise conv on the gpu), could be added to this PR based on discussion and support.
The latter because this is prolly out of depth at this point in time and will require some more diggin in.
Hopefully @avik-pal can add some more hints on what should be done :).