-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add group support for convolutions #9
Conversation
Does this need to be rebased on NNlib? |
Not sure why? |
Sorry I misspoke, I didn't mean rebase, but depend on nnlib#master for ci to pick up the latest code. Otherwise LGTM! |
Also, should there be a release for NNlib, before this PR can be merged? |
Co-authored-by: Dhairya Gandhi <dhairya@juliacomputing.com>
NNlib 0.7.25 is in the process of being registered. The compat bound in Project.toml should be updated here |
test/conv.jl
Outdated
end | ||
gputest(NNlibCUDA.∇conv_bias!, db, dy, checkgrad=false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you mind removing the function and test from here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the function is needed to test the gpu method of the same function we have in NNlibCUDA
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So do we keep the test and function?)
I had to add a dummy commit to retrigger buildkite |
buildkite didn't pick up the new NNlib version |
This PR adds support for groups in convolutions. It relies on this PR, so I'll mark it as draft for now.