-
-
Notifications
You must be signed in to change notification settings - Fork 609
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 Parallel layer #1462
Add Parallel layer #1462
Conversation
Also, I'd like to add that |
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.
Thanks, do we also want to add the functionality from having multiple arguments in the forward pass as in parallel execution and reduction? Seems like there's some additional functionality in #1289
Adding multiple input arguments would be like |
Actually on second thought, maybe we shouldn't have the multi-arg version? It complicates the semantics on |
@DhairyaLGandhi any decision on keeping vs excluding multiple input arguments for the layer? I would recommend excluding. |
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 vararg AbstractArray method is the one I had in mind. I would also want to see some addition to the docs that talks about implementing the Join
and Split
because I feel we would inevitably have questions about wanting them.
Otherwise, lgtm
What about |
I'm in favour of retaining the struct because it's a very common case, but rewriting it in a follow up |
@darsnack I was hoping the docs would show how to implement |
Co-authored-by: Atiyo Ghosh <atiyo@users.noreply.github.com>
Co-authored-by: Michael Abbott <32575566+mcabbott@users.noreply.github.com>
Co-authored-by: Michael Abbott <32575566+mcabbott@users.noreply.github.com>
Co-authored-by: Dhairya Gandhi <dhairya@juliacomputing.com>
Co-authored-by: Dhairya Gandhi <dhairya@juliacomputing.com>
1f60dfa
to
377c977
Compare
Note that |
@DhairyaLGandhi bump |
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.
Lgtm, thanks @darsnack
bors r+ |
@bhvieira Currently, it is still in Flux as it was before. My plan was not to deprecate it but to define: SkipConnection(layers, connection) = Parallel(connection, layers, identity) I think the |
@darsnack Yeah, I deleted my comment before seeing yours actually, that's what I had in mind! Good job btw! |
Build succeeded: |
Can we add follow on testing on the GPU, that seems to have been missed? My bad. @darsnack |
Since #1289 stalled, I have added an implementation of
Parallel
with some of the changes we discussed during ML calls. This version excludes most of the structural layers in #1289 likeJoin
,Split
, andNop
. I also added the ability for the user to specify the reduction operator. If it is acceptable, I would like to remapSkipConnection
toParallel
(not a deprecation exactly).The reason for submitting this PR now is because I am creating pre-trained weights for the networks in FluxML/Metalhead.jl#70, and there is a lot of code that can be replaced with a
Parallel
. So, I'd like to haveParallel
in Flux before continuing with training to make the process easier.PR Checklist
cc @CarloLucibello