Closed sethaxen closed 3 years ago
Ouch, thanks for letting me know about this. This is high priority, obviously. I'll dig into it and let you know what I find.
Since the code it's calling is now so simple,
function sample(rng::AbstractRNG,
m::ConditionalModel,
config::DynamicHMCConfig,
nsamples::Int=1000,
nchains::Int=4)
ℓ(x) = Soss.logdensity(m, x)
tr = xform(m)
chains = newchain(rng, nchains, config, ℓ, tr)
sample!(chains, nsamples - 1)
return chains
end
I wonder if the problem might be in SampleChainsDynamicHMC
Soss logdensity looks ok:
xx = range(-10,10,length=10000)
dens = [MeasureTheory.density(mod() | (y=0.2,), (μ=x,)) for x in xx]
ecdf = cumsum(dens)
ecdf ./= last(ecdf)
plot!(xx,ecdf, label="normalized cumsum of Soss density evaluations")
xlims!(-2,3)
In DynamicHMC here, mcmc_next_step
is used like this:
for i in 1:N
Q, tree_statistics[i] = mcmc_next_step(steps, Q)
chain[i] = Q.q
report(mcmc_reporter, i)
end
But SampleChainsDynamicHMC is currently using it like this:
function SampleChains.step!(chain::DynamicHMCChain)
Q, tree_stats = DynamicHMC.mcmc_next_step(getfield(chain, :meta), getfield(chain, :state))
end
So I think the problem might be that we're failing to update the state.
Think I got it
What was the problem?
GitHub is collapsing it, but https://github.com/cscherrer/SampleChainsDynamicHMC.jl/issues/12#issuecomment-880293340
I had to make the state mutable. Using a zero-dimensional array just to try it out, but I haven't benchmarked it vs other approaches.
Soss is failing to target the correct (analytically known) posterior in the following simple example using SampleChainsDynamicHMC. I ruled out any (obvious) bugs in MeasureTheory and DynamicHMC by using the two of them directly to sample from the same posterior.