Improve performance of `values_as_in_model`

[penelopeysm]

Right now values_as_in_model always re-evaluates the model.

DynamicPPL.jl/src/values_as_in_model.jl

Lines 125 to 129 in 2074657

	function values_as_in_model(model::Model, include_colon_eq::Bool, varinfo::AbstractVarInfo)
	varinfo = setaccs!!(deepcopy(varinfo), (ValuesAsInModelAccumulator(include_colon_eq),))
	varinfo = last(evaluate!!(model, varinfo))
	return getacc(varinfo, Val(:ValuesAsInModel)).values
	end

This is obviously inefficient. We should:

1. Make ValuesAsInModelAccumulator a default one. It costs almost zero time to do this. Not sure if this will meddle with type stability, but if it does I imagine it should be possible to find a workaround. (cough cough VarNamedTuple...)
2. Make a new method values_as_in_model(::VarInfo) that just takes the values out from the accumulator.
3. Check for usage of values_as_in_model(model, vi) in the code base that can be simplified to just values_as_in_model(vi).

I would also prefer renaming one of the two to something else to indicate that one does not evaluate and the other re-evaluates.

[mhauru]

For what it's worth, I've found values_as_in_model to be a confusing name. I remember when I was newer to the codebase, every time I saw it I was left with an unpleasant feeling of not understanding what is going on, because it begged the question of what might the default behaviour be, if as_in_model has to be explicitly specified. "As not in the model? What does that mean?" I don't have a good proposal for a better name though. Maybe it's okay for the name to be confusing, if the thing it does is inherently confusing. Which I think it is, because it is unintuitive that you need to do this at all, rather than just getting the values from the varinfo.

[penelopeysm]

I agree, I am all for renaming it ;)

[penelopeysm]

Tor often uses realisation (https://en.wikipedia.org/wiki/Realization_(probability)), but I wonder if 'unlinked values' is maybe more direct, since 'values' may be either linked or unlinked.

Personally my favourite would just be to use 'values'. Everything that is currently called 'values' should be called 'internal values', or similar, where 'internal' means 'possibly linked or unlinked'.

I find it counterintuitive that something like values_as(vi, NamedTuple) can return linked values.

[mhauru]

But aren't there also cases like a VarInfo that has x, y, and z and a model like

@model function f()
    x ~ Normal()
    if x > 0
        y ~ Normal()
    else
        z ~ Normal()
    end
    return nothing
end

for which values_as_in_model returns either x and y, or x and z, but never all there?

[penelopeysm]

That thought made me a little depressed, and then I thought about it, and actually it would never be possible to return all three, because values_as_in_model isn't extracted from the VarInfo's internal metadata storage but rather the accumulator.

The accumulator is emptied on every new model evaluation and only pushed to when we encounter a tilde statement, so after any model evaluation, the accumulator will either have x, y or x, z but not all three.

(Just so I'm clear, that is the intended behaviour, right? That's also what the current values_as_in_model does.)

[mhauru]

Sorry, rereading it now I was super unclear. That is indeed intended behaviour. The point of bringing up the above example was to point out that values_as_in_model returns something different from what you might expect from something called values(model, vi) with a VarInfo with x, y, and z in it. I take the _as_in_model part to essentially say that in the above example you would only get x and y or x and z but never all three.