Turing.Inference.Transition may fail for varinfos with non-default accumulators

[penelopeysm]

In here:

Turing.jl/src/mcmc/Inference.jl

Lines 162 to 183 in 0eb8576

	function Transition(
	model::DynamicPPL.Model, vi::AbstractVarInfo, stats; reevaluate=true
	)
	# Avoid mutating vi as it may be used later e.g. when constructing
	# sampler states.
	vi = deepcopy(vi)
	if reevaluate
	vi = DynamicPPL.setaccs!!(
	vi,
	(
	DynamicPPL.ValuesAsInModelAccumulator(true),
	DynamicPPL.LogPriorAccumulator(),
	DynamicPPL.LogLikelihoodAccumulator(),
	),
	)
	_, vi = DynamicPPL.evaluate!!(model, vi)
	end
	# Extract all the information we need
	vals_as_in_model = DynamicPPL.getacc(vi, Val(:ValuesAsInModel)).values
	logprior = DynamicPPL.getlogprior(vi)
	loglikelihood = DynamicPPL.getloglikelihood(vi)

getlogprior(vi) will fail if vi does not have a LogPriorAccumulator.

This cannot happen right now because vi is always constructed by Turing (there is no way to change what varinfo is used for sampling .... welll .... technically it is possible with default_varinfo(); but generally no). So it is not possible for this error to ever occur.

However, it's not future proof, and it's generally not ideal. For example, in principle one might want to sample with Prior() but without a likelihood accumulator (after all it's just prior sampling). But Prior then uses reevaluate=false, which means that if the varinfo doesn't have a likelihood acc, it will fail at getloglikelihood.

[penelopeysm]

I think the correct solution here is to remove the logprior and loglikelihood fields from Transition, and add a new method getstats(::Transition) (or modify whatever it's called right now, I think it's getstats_with_lp), and make htat take transition.stats but also merge it with logprior and loglikelihood if they can be found (and logjoint if both can be found).

[penelopeysm]

DynamicPPL.ParamsWithStats should cleanly solve this, although it'll take a while to get it in Turing.