-
Notifications
You must be signed in to change notification settings - Fork 228
Description
In these lines, component samplers (usually) return a tuple of (Turing.Inference.Transition, SamplerState)
.
The construction of Turing.Inference.Transition
requires a model re-evaluation (because it runs values_as_in_model
, and after #2630, also recalculates logp).
However, in the Gibbs sampler we just throw that away:
Lines 442 to 451 in d0510b1
# Take initial step with the current sampler. | |
_, new_state = step_function( | |
rng, | |
conditioned_model, | |
sampler; | |
# FIXME: This will cause issues if the sampler expects initial params in unconstrained space. | |
# This is not the case for any samplers in Turing.jl, but will be for external samplers, etc. | |
initial_params=initial_params_local, | |
kwargs..., | |
) |
Lines 678 to 679 in d0510b1
# Take a step with the local sampler. | |
new_state = last(step_function(rng, conditioned_model, sampler, state; kwargs...)) |
We should either:
(1) Avoid triggering the Transition re-evaluation unless it's needed (not obvious how to do it).
(2) Investigate whether we can use the Transition return value to avoid the re-evaluation in setparams_varinfo!!
.
Note that either of these options may require storing the new varinfo itself in the Turing.Inference.Transition
struct.