FluxML/Functors.jl

Correct way to reconstruct an instance of an object

willtebbutt opened this issue · 6 comments

The following (unsurprisingly) yields an error:

struct Foo
    a::Float64
    b::String
end

@functor Foo (a,)

x, re = functor(Foo(5.0, "hi"))

julia> re(x)
ERROR: MethodError: no method matching Foo(::Float64)
Closest candidates are:
  Foo(::Float64, ::String) at REPL[34]:2
  Foo(::Any, ::Any) at REPL[34]:2
Stacktrace:
 [1] (::var"#22#23")(::NamedTuple{(:a,),Tuple{Float64}}) at /Users/willtebbutt/.julia/dev/Functors/src/functor.jl:12
 [2] top-level scope at REPL[37]:1

You can imagine this kind of thing cropping up, where only a subset of the fields of an object are considered "parameters", but you do need to have access to the others to make an instance of the object.

Is the intended way to handle this to define my own method of functor as follows:

function Functors.functor(x::Foo)
    function reconstruct_Foo(xs)
        return Foo(xs.a, x.b)
    end
    return (a = x.a,), reconstruct_Foo
end

julia> x, re = functor(Foo(5.0, "hi"))

julia> re(x)
Foo(5.0, "hi")

julia> fmap(Float32, Foo(5.0, "hi"))
Foo{Float32}(5.0f0, "hi")

?

There are a couple of ways to handle this situation. Overloading functor is totally valid (although you probably want to overload functor(::Type{<:Foo}, x) rather than functor(x::Foo); the former method is used under the hood so that we can handle both a Foo and its gradient the same way).

There are a couple of other ways. First, you can just ignore Strings when you map over parameters, assuming only numbers and numerical arrays are relevant to what you're doing. e.g. fmap(x -> x isa Array{<:Number} ? f(x) : x, model).

Another option is to create a wrapper around functor, e.g. in Flux trainable falls back to the functor definition but can also be easily overloaded by layers. Then params walks over the output of trainable, not functor.

Hope that helps; unfortunately it's a bit use-case-dependent but I'd be happy to look at what you're doing in more detail if that would be useful.

Sounds good to me. It was more that I just want to know what the options are / whether any are actively discouraged.

Any chance some of this advice might appear in the readme?

Might be useful to quickly link these issues in the readme/code?

Resolved by #4

Another option is to create a wrapper around functor, e.g. in Flux trainable falls back to the functor definition but can also be easily overloaded by layers. Then params walks over the output of trainable, not functor.

Would it make sense to move trainable here as well? In this way there would be a "framework agnostic" way of specifying what are the parameters and what are the trainable parameters. Otherwise, Flux is a bit of a heavy dependency (in terms of load time) to just specify that some parameters are not trainable.

Would it make sense to change @functor such that it defines the version suggested by @willtebbutt above (based on the type)? I would guess that in most cases that's what a user would expect and implement.