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

Correct way to reconstruct an instance of an object #3

Closed
willtebbutt opened this issue May 7, 2020 · 6 comments
Closed

Correct way to reconstruct an instance of an object #3

willtebbutt opened this issue May 7, 2020 · 6 comments

Comments

@willtebbutt
Copy link
Member

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")

?

@MikeInnes
Copy link
Member

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.

@willtebbutt
Copy link
Member Author

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?

@MikeInnes
Copy link
Member

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

@willtebbutt
Copy link
Member Author

Resolved by #4

@piever
Copy link

piever commented Jun 27, 2020

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.

@devmotion
Copy link

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.

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

4 participants