Clarification of fieldvalues, API and related packages

[oschulz]

According to the docs the behavior of constructorof should be obj == constructorof(obj)(fieldvalues(obj)...). But when specializing constructorof, which fieldvalues function should be targeted - is it IterTools.fieldvalues?

[jw3126]

You are touching an awkward point here. I think there is currently no precise definition of fieldvalues. Personally, I think fieldvalues should always be semantically equivalent to fieldvalues(obj) = Tuple(getfield(obj, f) for f in fieldnames(typeof(obj)). E.g. it exposes all private details of an object. If you want privacy hiding, then setproperties and getproperties should be used.

cc @rafaqz @tkf

[oschulz]

I really like the lightweight approach of ConstructionBase - I have a need for something like this to improve ForwardDiffPullbacks.jl. But I do need both directions (decomposition and reconstruction) and if people specialize constructorof for a type, there obviously needs to be a clearly defined way how to extract ctor args (field values) given an existing object - a way that can be specialized for the given type as well.

We have several things in this direction scattered across the ecosystem at the moment (Functors.jl seems closely related but incompatible, for example). It would be so nice so have a consensus (analog to ChainRulesCore, in a way) for de/reconstruction that can power things like generic flatten/unflatten, and so on.

Question is, what should getfieldvalues return for types with internal fields (precomputed stuff, etc.) that have no semantic meaning and are not part of the typical user-facing ctors? I'd image those should be excluded? PDMats.PDMat might be a nice test case as it's field values are highly redundant (I would argue the actual content is just the chol field, as the object can't be reconstructed without numerical problems from the mat field).

[jw3126]

Question is, what should getfieldvalues return for types with internal fields (precomputed stuff, etc.) that have no semantic meaning and are not part of the typical user-facing ctors? I'd image those should be excluded?

I am not sure, it depends on the job description of fieldvalues + constructorof. I imagine these as a low level ultimate truth way to compose + decompose stuff. Similar to Base.getfield, you should not overload fieldvalues, or at least not change its semantics. A typical use case for me would be moving an object to GPU and there I really want to move everything no matter if it is private or not.

Privacy and fancy semantics are the job of setproperties + getproperties IMO.

[oschulz]

Similar to Base.getfield, you should not overload fieldvalues, or at least not change its semantics.

That's what I meant - is constructorof intended to work as an "inverse" of a low-level thing like Base.getfield, or something higher level, more semantic, that may be specialized? I saw that ConstructionBase.jl is used by ModelParameters.jl, that's why I assumed this was about the semantic fields of types instead of all fields.

A typical use case for me would be moving an object to GPU and there I really want to move everything no matter if it is private or not.

Isn't that what Adapt.jl is for?

[oschulz]

that's why I assumed this was about the semantic fields of types instead of all fields.

I don't mean to say that having a tool that can reconstruct values from their low-level field values isn't very important as well (@piever, maybe StructArrays.jl could profit from constructorof - I think it currently has similar internal helper functions?). I was just wondering whether the intention was for it to be high-level or low-level.
'

[rafaqz]

Yes there is an awkward tension between getfield/getproperties in ConstructionBase. But I also lean towards just using all the private fields. For Adapt.jl, you can instead use e.g. Flatten.jl (also using constructorof) or my PR to Accessors.jl to replace all the Array in an object without defining a specific Adapt.jl method for it. Unfortunately current type instabilities in Base make this slower than it used to be, but theoretically if this is ever fixed, doing it with my PR to Acessors.jl should be the easiest way and essentially replace Adapt.jl.

[jw3126]

@rafaqz so should we state in the docs that fieldvalues(obj) = Tuple(getfield(obj, f) for f in fieldnames(typeof(obj))?

[rafaqz]

I guess we should, althoughmaybe warning not to actually write it that way.

[oschulz]

So built-in getfield, not getproperties, right?

[oschulz]

I wonder if Functors.jl could use constructorof and ... getfield ... as a default implementation for unkown types (I think it currently doesn't have one).

[jw3126]

So built-in getfield, not getproperties, right?

Yes, constructorof should take as arguments what you obtain from built-in getfield.

[rafaqz]

And yes Functors.jl could totally use constructorof as the default.

[oschulz]

And yes Functors.jl could totally use constructorof as the default.

@mcabbott, could you see that happening?

[piever]

I don't mean to say that having a tool that can reconstruct values from their low-level field values isn't very important as well (@piever, maybe StructArrays.jl could profit from constructorof - I think it currently has similar internal helper functions?).

Thanks for the ping. Yes, StructArrays could potentially use this as well. There may be a little bit of awkwardness, in that StructArrays allows to customize the tuple of fieldvalues (defaults to the tuple of getfield(v, i) for i in 1:fieldcount(T)), but constructorof(T) is definitely a reasonable fallback.