`hasvalue(p.params, vn, dist)`

[penelopeysm]

The three-argument hasvalue method, used inside InitFromParams:

DynamicPPL.jl/src/contexts/init.jl

Lines 113 to 118 in 8e359ce

	return if hasvalue(p.params, vn, dist)
	x = getvalue(p.params, vn, dist)
	if x === missing
	p.fallback === nothing &&
	error("A `missing` value was provided for the variable `$(vn)`.")
	init(rng, vn, dist, p.fallback)

is only really needed when we provide VarNames that are 'broken up' relative to how they appear in the model. Here's an example:

using DynamicPPL, Distributions, LinearAlgebra

@model function f2d()
    return x ~ MvNormal(zeros(2), I)
end

returned(f2d(), Dict(@varname(x[1]) => 0.0, @varname(x[2]) => 0.0))

On current main, this will print [0.0, 0.0]. But that's only because hasvalue(dict, vn, dist) can 'reconstruct' x from the x[1] and x[2] that we provided. If we replace this with hasvalue(dict, vn), then this will error with a message saying that x wasn't found.

For cases like returned, I actually really don't mind if the above errors. That Dict was obviously constructed by me to prove a point, and I can fix it by simply constructing it the correct way, i.e. Dict(@varname(x) => [0.0, 0.0]).

The only case where we unconditionally, absolutely, DO need hasvalue(dict, vn, dist) is when the dictionary is obtained from MCMCChains. That happens because MCMCChains 'proactively' splits the parameters up.

There are two things to investigate here:

1. Is hasvalue(dict, vn, dist) noticeably slower than hasvalue(dict, vn) for cases where the dictionary already has the correct form (i.e. Dict(@varname(x) => [0.0, 0.0]))? If the performance is roughly the same, then we can just leave it in and call it a day, since it is strictly an improvement.

2. If it isn't the same, then what we could do is to create a separate initialisation strategy which DOES use this, and use that only in MCMCChains. That would be something like InitFromMCMCChains. Then, to make InitFromParams faster, we could cut out the third argument from this.

   • Note that this solution would also mean that the hasvalue(p, vn, dist) definitions, which currently sit in an awkward AbstractPPLDistributionsExt, could be moved to MCMCChains itself.