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 Functors.jl? #439

Open
oschulz opened this issue Aug 16, 2021 · 7 comments
Open

Support Functors.jl? #439

oschulz opened this issue Aug 16, 2021 · 7 comments

Comments

@oschulz
Copy link
Contributor

oschulz commented Aug 16, 2021

I could be very handy if the tangent types would support Functors.functor - see FluxML/Functors.jl#20.

Would Functors.jl be lightweight enough to be acceptable as a dependency for CRC?

@oschulz oschulz changed the title Support Functors.jl Support Functors.jl? Aug 16, 2021
@ToucheSir
Copy link
Contributor

I know you moved this from the Functors tracker, but if anything the dependency should probably be the other way around (or handled by a glue package). Namely, we might want to add rrules for Functors.functor and related functions eventually.

@oxinabox
Copy link
Member

oxinabox commented Aug 16, 2021

This particular case is one of my examples for why we need conditional dependencies. JuliaLang/Pkg.jl#1285 (comment)

In short:
No, it would not make sense

We have no dependencies except Compat.jl (which is basically a stdlib at this point.)
And we won't take any.
Functors.jl would be a dependency.
Further it would add a transitive dependency on MacroTools.

10 packages directly depend on Functors.jl and 80 indirectly.
In contrast:
44 packages directly depend on ChainRulesCore, and over 1500 indirectly.
Of the 10 packages that depend on Functors.jl all but one or two depend directly or indirectly on ChainRulesCore.

It also doesn't hugely make sense.
One of the goals of ChainRulesCores types is that you shouldn't have to destructure them into some flat vector: they already model a vector space directly, and overload appropriately.
I get that for some optimisation packages (esp ones that call into C/Fortan) that is required though.

Ideally, Functors.jl would workout how to have implicit functors on things that overload map / iterate, which we do.
It feels like that should just work.

But failing that
Functors.jl should add a dependency on ChainRulesCore.jl.
Or if that can't happen, then a glue package that type pirates both.
To be fixed later by conditional dependencies.

@oschulz
Copy link
Contributor Author

oschulz commented Aug 17, 2021

We have no dependencies except Compat.jl (which is basically a stdlib at this point.)

I know you moved this from the Functors tracker, but if anything the dependency should probably be the other way around [...]
Functors.jl should add a dependency on ChainRulesCore.jl. [...]
We have no dependencies except Compat.jl

And the Functors.jl authors will probably argue that they have no depencency except MacroTools, which is also basically unavoidable as soon as you install enough packages to make ChainRulesCore useful. :-)

Both packages are very lightweight and naturally want to remain so - it's a conundrum ...

One of the goals of ChainRulesCores types is that you shouldn't have to destructure them into some flat vector

Maybe I'm misunderstanding something - isn't the fact that you don't need to flatten the main point of Functors?

Hm, what if Functors could be made even more lightweigh? DhairyaLGandhi , would it be conceivable to have a FunctorsBase package that only defines function functor end, and little else?

@oxinabox
Copy link
Member

oxinabox commented Aug 17, 2021

Even if Functors.jl was more lightweight we still would not add it.
Even if it was literally an empty package.
Regardless, lightweightness was only one of the ~3 reasons we don't want to.

Lightweightness, number of dependencies (especially since almost all Functors.jl dependents already depend on ChainRulesCore), and conceptualness.
To add another one: stability.
We are at 1.x, Fuctors is at 0.x.

I am sorry that this is a hard problem.

@oxinabox oxinabox reopened this Aug 17, 2021
@oxinabox
Copy link
Member

oxinabox commented Aug 17, 2021

oops didn't mean to actually close this.
The answer is a pretty hard no.
but I am happy to leave this issue open til you are happy to close it.

(edit: arg, why are those buttons right next to each other)

@oxinabox oxinabox reopened this Aug 17, 2021
@oschulz
Copy link
Contributor Author

oschulz commented Aug 17, 2021

Well, I just had a feeling that ChainRulesCore.Tangent and Functors.functor provided orthogonal concepts that could potentially be powerful together. But I'm not a maintainer of either package, so it's certainly not for me to say which way this should go.

I think think what Functors does (independent of it's maturity) may be, in a way, more fundamental than ChainRulesCore, since it basically allows for adding semantic (i.e. using the "meaningful" fields) introspection and decomposition/recomposition mechanism to types - it's certainly not the only API one could think up for doing so though, of course. But we do need a way of traversing both original types and their tangents in parallel, so that algorithms like SGD optimzers can do their thing on the leaves, right?

@ToucheSir
Copy link
Contributor

I'm not sure why Functors has a dep on MacroTools, opened FluxML/Functors.jl#22 to track that.

The biggest question remains one of stability. I don't think we're anywhere close to settling on a final API for Functors, and thus 1.0 will probably take a while to materialize.

RE convergence, I'm also not sure what Functors brings to ChainRulesCore that isn't covered by Tangent and ProjectTo already. Part of this comes back to the API design aspect: despite being a small Package, Functors wears a number of disparate hats right now:

  • Recursively mapping over structures. Like Adapt.adapt_structure, but more general. AIUI this was the original motivation for the package as a generalization of (the now defunct) Flux.mapleaves.-
  • (un)flattening of arbitrary structures into POJOs. This is similar to ProjectTo, and possibly somewhere we can pull on in the future.
  • Fancy traversal of a DAG of arbitrary structures. This is quite limited right now and IMO the weakest point of the library. exclude and custom walk functions help, but they're inelegant solutions at best.

It may be that there is no good abstraction for addressing all of these goals satisfactorily, but we certainly won't know until we try :)

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

No branches or pull requests

3 participants