add fcollect

[CarloLucibello]

Follow up to the discussion in FluxML/Flux.jl#1444.

I tried to give maximum flexibility to the interface.

Notice that it makes sense to pass a 2-args function f(v, vs) instead of a 1-arg f(v), this is because applying a 1-arg f is quite simple a-posteriori

f.(fcollect(m))

Possible applications:

• Retrieve leaves from a model:

filter(isleaf, fcollect(m))

• Retrieve all nodes except leaves

filter(!isleaf, fcollect(m))
# or
fcollect(m; recur = (v, vs) -> !isleaf(v))

• Retrieve parents with all leaf children

a = fcollect(m; f = (v, vs) -> all(isleaf, vs) ? v : nothing)
filter(!isnothing, a)

[darsnack]

I feel like the two-arg version is because we don't have something like children(x) = functor(x)[1] in Functors.jl. I would prefer syntax like:

a = fcollect(m; f = x -> all(isleaf, children(x)) ? x : nothing)
filter(!isnothing, a)

[CarloLucibello]

I feel like the two-arg version is because we don't have something like children(x) = functor(x)[1] in Functors.jl. I would prefer syntax like:

a = fcollect(m; f = x -> all(isleaf, children(x)) ? x : nothing)
filter(!isnothing, a)

Yeah. I think we can remove the f argument altogether and let people handle further processing downstream.

filter(x -> all(isleaf, children(x)), fcollect(m))

What do you say?

[darsnack]

Yeah I think that would match the name fcollect better. No reason to include f just cause fmap has it (which was my original thinking).

[darsnack]

We'll want to settle on the same name for recurse vs. predicate. I think I am okay with recurse.

[CarloLucibello]

We'll want to settle on the same name for recurse vs. predicate. I think I am okay with recurse.

I'd go with recurse, conveys more intuition, the term predicate is very generic. Although the semantic is opposite, recurse == !predicate.

[darsnack]

Looking good to me. Will probably want @ToucheSir's review since he has more wisdom on Functor-y things than me.

[darsnack]

exclude is good. Will use that instead of predicate.

[CarloLucibello]

I changed my mind, i think it is more convenient to specify what to exclude from the recursion instead of what to include, so removed recurse in favor of exclude.

[ToucheSir]

LGTM, this is essentially Flux.destructure without trying to shove everything into one vector. One thing we could look at next is creating an iterable version. Then fcollect could be expressed simply as collect(fiter(m, ...)).

[CarloLucibello]

Does anyone have merge rights?

[darsnack]

I don't think so

[CarloLucibello]

we good?