Add getparams and setparams!! following AbstractMCMC v5.5 and v5.6

[sunxd3]

ref: TuringLang/Turing.jl#2367

[sunxd3]

the failed test is not related to this PR, @yebai maybe you already have an answer to why it fails?

[yebai]

@sunxd3 This is due to some changes in Julia’s handling of script entry functions. Can you try to fix that?

[torfjelde]

The current impl is going to lead to incorrect log-density values being used in the transition. IMO we shouuld do the safe thing by default here.

[torfjelde]

One dangerous aspect there is that state.transition caches the logjoint and gradient computations. So when you use the @set macro here, you're going to also keep the cached log-joint and gradient computation, which will then be out of sync with the parameters.

If you then naively pass this transition somewhere, say, into the next step call, AHMC.jl will use the incorrect logjoint eval in the MH step.

IMO the safe way is to use the explicit constructor of PhasePoint I believe without passing in the cached values. This should result in receomputation of this.

[sunxd3]

just realized that model is not passed into setparams!!, I think for now, we only set the parameters, then when use the transition, logp should be recomputed. (we can also later introduce some like setlogp or compute_logp etc.) I'll add a comment in the code

[yebai]

Agreed with @torfjelde's concern. Can we add logdensitymodel to

• setparams!!(state, logdensitymodel, params)
• getparam(state, logdensitymodel).

where logdensitymodel follows the LogDensityProblem interface. This would allow us to recompute the model's log probability (i.e., recompute_logp) inside these setparam!! functions on demand, which Turing's new Gibbs sampler uses.

[sunxd3]

let's delay a bit to see through TuringLang/AbstractMCMC.jl#150

[torfjelde]

The fix to setparams!! doesn't work unfortauntely. We need access to the model here, which we don't have.

IMO we should add model as an argument to the AbstractMCMC.jl interface. Otherwise there's no way we can do stuff like update the gradient information, which is an issue we're facing both here and in AdvancedMH.jl.

[torfjelde]

model isn't defined here 😕