Skip to content
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

Support NamedTuples for Container Layers #1680

Closed
ToucheSir opened this issue Jul 29, 2021 · 4 comments · Fixed by #1681
Closed

Support NamedTuples for Container Layers #1680

ToucheSir opened this issue Jul 29, 2021 · 4 comments · Fixed by #1681

Comments

@ToucheSir
Copy link
Member

This came up in FluxML/Metalhead.jl#70 and surprised me, because I thought we supported this already!

Other than Chain, are there any other layers we'd want to support this? The only other one that uses a tuple field is Parallel, but since it doesn't forward methods to .layers I'm not sure this is applicable.

Another thing using NamedTuples would open up is the possibility of defining aliases for Chain. e.g.

const ResNetBlock{C1,C2,N} = Chain{NamedTuple{(:conv1, :conv2, :norm), Tuple{C1, C2, N}}}
@darsnack
Copy link
Member

I think this in combo with @mcabbott's "big show" would make models really readable. Like big show can print the names alongside the other info.

Is forwarding methods conceptually linked here (am I missing something)? For Parallel it would be nice to name the branches. I could see this being really useful for inception modules.

@DhairyaLGandhi
Copy link
Member

DhairyaLGandhi commented Jul 29, 2021

That works already in the sense that we can optionally store named tuples, what was spoken about in the discussion was how to leverage it for larger models.

Forwarding indexing would be needed. The printing is really to have custom show methods, since the Chain aren't as meaningful as the concept they represent.

@darsnack
Copy link
Member

Chain is one of those weird layers in that its types/constructor are extremely restrictive:

struct Chain{T<:Tuple}
layers::T
Chain(xs...) = new{typeof(xs)}(xs)
end

@darsnack
Copy link
Member

I think Parallel does forward indexing to the branches?

bors bot added a commit that referenced this issue Aug 4, 2021
1681: Support NamedTuples for Chain + Parallel r=mcabbott a=mcabbott

Closes #1680, WIP. Todo list includes:

- [x] add Parallel too
- [ ] ~~worry about whether any of this will upset Zygote, like FluxML/Zygote.jl#909 or, kick that can down the road.
- [x] add tests

Co-authored-by: Michael Abbott <[email protected]>
bors bot added a commit that referenced this issue Aug 4, 2021
1681: Support NamedTuples for Chain + Parallel r=DhairyaLGandhi a=mcabbott

Closes #1680, WIP. Todo list includes:

- [x] add Parallel too
- [ ] ~~worry about whether any of this will upset Zygote, like FluxML/Zygote.jl#909 or, kick that can down the road.
- [x] add tests

Co-authored-by: Michael Abbott <[email protected]>
@bors bors bot closed this as completed in dbb9f82 Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants