JuliaStats / Klara.jl

MCMC inference in Julia
Other
166 stars 38 forks source link

Is Hermitian() needed before call to chol() in RAM? #147

Closed papamarkou closed 7 years ago

papamarkou commented 7 years ago

@omalled, I opened this issue as a placeholder, and will look into it soon, once fully done with the NUTS sampler.

Your example:

import Klara
logtarget(x) = -sum(x .^ 2)
mcparams = Klara.BasicContMuvParameter(:p, logtarget=logtarget)
model = Klara.likelihood_model(mcparams, false)
initvals = zeros(3)
sampler = Klara.RAM(fill(1e-1, length(initvals)))
mcrange = Klara.BasicMCRange(nsteps=1000, burnin=100, thinning=1)
mcparams0 = Dict(:p=>initvals)
job = Klara.BasicMCJob(model, sampler, mcrange, mcparams0, tuner=Klara.VanillaMCTuner())
Klara.run(job)
papamarkou commented 7 years ago

@andreasnoack, do you know what changes have taken place in julia v0.5 in relation to Cholesky factorizations? Examples on which a simple chol(A) would do in v0.4 now return an error accompanied by a suggestion to call chol(Hermitian(A)).

What internal algorithmic change has been made in chol()? Is the anticipated fix to call chol(Hermitian()) in the relevant MCMC routines?

papamarkou commented 7 years ago

@omalled, I called chol(Hermitian()) as you initially proposed, and the Klara code for RAM (and SMMALA) seems to be cross-compatible with Julia v0.4 and v0.5, oddly without even needing to explicitly use Compat. Let me know if you still get any error messages - I tried your minimal example and it seems to be working both with v0.4 and v0.5 using the latest Klara commit.

omalled commented 7 years ago

Thanks, @scidom. This fixed the example I posted and the original problem.