VarName enhancements

[phipsgabler]

VarName is working pretty well in its current form, but the logic for subsumption could be defined a
bit more explicitely. Some helper methods are certainly needed in order to properly support
subsumption in the dictionary interface, and would become useful elsewhere.

In the long term: what about more complex things, like getproperty-based variables?

Also, there is the idea of having a separate VariableSpace structure: TuringLang/DynamicPPL.jl#10. This could go here as well.

[phipsgabler]

I am pretty convinced that VarName should actually be a restricted form of a lens.

• @varname(x) == VarName{:x}()
• @varname(x[1]) == VarName{:x}() ∘ Indexed((1,))
• @varname(x.y) == VarName{:x}() ∘ Property{:y}()

[devmotion]

In SciML we still use Setfield.

[cscherrer]

@torfjelde this might help:
JuliaObjects/Accessors.jl#9

[torfjelde]

Yeah I saw that:) Honestly seems like there's not much to gain from adopting Accessors.jl instead of Setfield.jl? So IMO we should just go with Setfield.jl and then make the move once Accessors.jl becomes the go-to.

[cscherrer]

The big advantage I saw is that in a package's early stages it's much easier to have a part of the design process, so you can make sure the package will fit your use case. Setfield is very nice but it's now frozen.

Typical stability vs innovation trade-off I guess 🙂

[torfjelde]

Typical stability vs innovation trade-off I guess 🙂

True! I'd say that for projects under TuringLang we generally lean towards the latter though. Plus we can still provide useful usecases/information to Accessors.jl even though we're using it's predecessor 👍

[torfjelde]

We often have to deal with AbstractArray{<:VarName{sym}} where {sym} in DPPL, and so it would be nice to have a way of doing that. In DPPL we're (starting from TuringLang/DynamicPPL.jl#259) doing a bit of type-piracy with

AbstractPPL.getsym(::AbstractArray{<:VarName{sym}}) where {sym} = sym

[devmotion]

Adding from the discussion in DynamicPPL: probably we could avoid it by working with arrays of indices instead. But then (better) support for arrays of indices would be needed.

[phipsgabler]

Copying my comment from over there:

Maybe a there's a lattice structure or something to varnames, it kind of feels like it, then we'd have join/meet and the meet is the collection of indices.

And yes, in theory something like VarName{x}((([1, 3],),)) already works, but I don't know how stable it is wrt. subsumption. I think it's mentioned as disallowed in the docstring.

[torfjelde]

Btw, is subsume even used for anything these days or are we just using DynamicPPL.subsume_string?

It clearly has it's use though (and ideally we wouldn't be using strings), but just pointing out that innovating on the subsume functionality probably won't break anything.

[phipsgabler]

It's required for inspace, which I guess is used all over the place? Or has that changed in the past year?

[torfjelde]

Ah that makes sense! For some reason my searches didn't give me any results...

[phipsgabler]

Reviving this discussion a big... we're about to implement this: #26 (review), and it all turns out rather nice (free hugs to @torfjelde, please). Some technical aspects/questions remaining:

• For now, as discussed above, we stick to Setfield.jl. Moving to Accessors.jl, if necessary, shouldn't be much of a problem and mostly involve renaming, I think.

• Related to the recent begin/end quabblings: in principle, this is resolved nicely by reusing Setfield.jl's parsing. However, usages of them return a DynamicIndexLens containing the closures with firstindex/lastindex, instead of a plain IndexLens. Therefore, we have added a concept of "concretization" -- a VarName can be "concretized" on an object by evaluating all the dynamic parts and converting down to an IndexLens. This is now automatically done the expansion of @varname when the new second argument is true, so that's all that is needed to be changed in DPPL in that respect.

  Still, the question arises how much to distinguish "dynamic" from "concrete" VarNames. Should there by a type tag? A trait with a @generated implementation? Runtime errors? The reason is that it might be practical to have more typing support for concreteness, since only concrete lenses make sense as dictionary keys in things like VarInfo. (DynamicIndexLenses are not even meaningfully comparable for equality.)

  In any case, we will still allow the construction of non-concrete VarNames, if only for generality and possible future usage.

• We should also contemplate how completely we should embrace (and communicate, and export) the fact that VarNames are then, in many respects, lenses. Like, there's alreay the composition method, and we should have get and set working, too.

  The only addition we make is that by the additional name, the varnames can be used as dictionary keys for parts of structures. From some point of view, this is really a special "VsymLens" composed with the indexing lens, if we view it as a lens for VarInfo (the "VsymLens" can be neither a PropertyLens, nor an IndexLens with symbol key, because we don't know beforehand how a VarInfo is implemented). On the other hand, this perspective does not work so well when applied to variables (get(x, @varname(x[1]))).

  Should get(obj, vn::VarName{sym}) be get(obj, compose(PropertyLens{sym}(), vn.indexing), or just get(obj, vn.indexing)?

[phipsgabler]

Uh @torfjelde I just had an idea. What about

getsymlens(::Type{PropertyLens}, vn::VarName{sym}) = PropertyLens{sym}()
getsymlens(::Type{IndexLens}, vn::VarName{sym}) = IndexLens((sym,))
getsymlens(vn::VarName) = getsymlens(PropertyLens, vn)

[torfjelde]

So I don't see an immediate usecase here. In DPPL we only really use the get that I have in #26 (excluding getindex(vi, vn) since we can't cover that properly in APPL anyways), and we wouldn't be using getsymlens(IndexLens, vn) anywhere. It's also unclear to me if that's really what you want when working with, say, a Dict, for example you might prefer to have the entire vn as a key or just Symbol(vn) as key rater than the sym only.

Because of this I'd say let's just defer this sort of handling to the packages that want to use it in special ways, e.g. getindex(vi, vn) in DPPL. At least for now. If more common behavior emerges as we start using more, then it might make more sense to add something like getsymlens?