Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add namedparams #1144

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

add namedparams #1144

wants to merge 5 commits into from

Conversation

AStupidBear
Copy link
Contributor

No description provided.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@bhvieira
Copy link
Contributor

Tests will also be necessary here.

@bhvieira
Copy link
Contributor

Also, if the parameter has no name, does it make sense to return :nothing like in the example?

@AStupidBear
Copy link
Contributor Author

Do you mean we should filter out parameters without names?

@DhairyaLGandhi
Copy link
Member

Shouldn't the pairs be name => param? Just a thought.

Also, what is the motivation behind using this more generally?
We already support things like gs[param], is it supposed to augment that? In that case you'll want to add in layer information, to distinguish between two layers of the same type

@AStupidBear
Copy link
Contributor Author

@dhairyagandhi96 namedparams is useful when you want to add penalties only for weights but not biases.

@AStupidBear
Copy link
Contributor Author

In that case you'll want to add in layer information, to distinguish between two layers of the same type

Yes, layer information should be added.

@AStupidBear
Copy link
Contributor Author

Tests and docs are added. Incorporating layer information is hard in the current setting of params.

@AStupidBear
Copy link
Contributor Author

Any idea?

@ToucheSir
Copy link
Member

I think this would be better served by taking gradients WRT the model directly, which will give you back a struct of the same shape. In general we've been trying to move away from the bag of params approach to leverage functors more, so IMO the effort is better spent there.

@CarloLucibello
Copy link
Member

there are some use cases for this for model inspection, but the pairing should be name => param.
With names supposedly in the form chain.layers[3].W.
Not easy to implement though.

cf #1444 as slightly related

@darsnack
Copy link
Member

I understand the use case for this, and I think it is natural syntax coming from other languages. But I feel that the best way to write code that would use namedparams is to use dispatch instead. In other words, you are usually trying to map a function onto a model, and you want to exclude bias or something. Easy enough to use Functors.jl and specialize the function on atomic layers like Dense or Conv to exclude the bias. Or you can use pure dispatch though that's a bit less generic. I am happy to be corrected if I am missing something though.

I agree with @ToucheSir's point and would add that it would be better to explain in the docs how to do such things. Like a "mapping function over parameters" tutorial.

@ToucheSir
Copy link
Member

AIUI this would require some additional infrastructure to work as an input to gradient? Is that worth it if users can already just pass the model itself and get back something that can be traversed exactly the same way? No need to worry about nesting or what string to use to join together nested path names.

@darsnack darsnack mentioned this pull request Feb 12, 2021
4 tasks
@ToucheSir
Copy link
Member

Upon revisiting this, I think it would be nice to have a function that returns structured model parameters as a (named)tuple, much like what gradient returns when one uses explicit params. However, I don't believe this is the way to do that, and most of the use-cases that don't have to do with structured params are covered by modules already.

@darsnack
Copy link
Member

The suggestion by @ChrisRackauckas in #1613 to use ComponentArray to represent params/grads seems like a better route. Maybe in combination with Functors.jl to generate said ComponentArray?

@ToucheSir
Copy link
Member

I'm not sure Functors should be in the business of knowing what a ComponentArray is, but perhaps Flux could thread everything together if the dependency isn't too onerous. I could see it being quite compelling for destructure, for example.

@darsnack
Copy link
Member

Yeah, this would be Flux using Functors + ComponentArrays to provide something equivalent to namedparams, not making Functors use ComponentArrays.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants