francescoalemanno / KissABC.jl

Pure julia implementation of Multiple Affine Invariant Sampling for efficient Approximate Bayesian Computation
MIT License
29 stars 8 forks source link

Non-integer parameter sample from discrete prior #30

Open mauricelanghinrichs opened 3 years ago

mauricelanghinrichs commented 3 years ago

Hey!

first, thanks a lot for this package! I was particular happy to see the support for (mixed continuous/) discrete priors, and the smc method seems to work well in my initial runs so far. However, I noticed a potential bug when using the ABCDE method (not yet released, current master branch of KissABC v3.0.1). When using a discrete prior the parameters from that prior are (sometimes) non-integer values (in contrast to smc).

Would be awesome if someone could take a look, as I'm excited to try out also the DE method! The following code produces the behaviour for me (Julia 1.6, macOS, Distributions v0.23.12):

using KissABC
using Distributions
using Random

Random.seed!(1)
const p1 = 80.0
const p2 = 10.0
const targetdata = rand(Normal(p1, p2), 1000)

function dist((μ,σ))
    # isinteger(μ) && @info("Indeed integer ", μ)
    # isinteger(μ) || @warn("Found non-integer ", μ)

    # Int() will throw error
    simdata = rand(Normal(Int(μ),σ), 1000)
    d1 = mean(simdata) - mean(targetdata)
    d2 = std(simdata) - std(targetdata)
    hypot(d1, d2)
end

# mixed discrete/continuous prior
const prior=Factored(DiscreteUniform(0.0, 200.0), Uniform(0.0, 50.0))

# smc runs fine
const res_smc = smc(prior, dist, verbose=true)

# ABCDE throws error like InexactError: Int64(22.32...)
const res_DE = ABCDE(prior, dist, 0.3, nparticles=100, generations=100)

Best

pcjentsch commented 3 years ago

I am not the developer of this package, but I have been using it for the past year or so. The ABCDE method was in v1 of this package but was removed because the posteriors it produces have "bias" (see this discourse post by the author). They mention that this bias can be overcome by running it for additional iterations.

The author added the method back, undocumented, after I pointed out that it performs far better on some of the big disease model problems I work on.

I haven't used it with mixed priors, so I cannot comment on your problem, I think it's probably a misplaced promotion thing. I hope this context was helpful though! If you know more about the differential evolution method and it's implementation then I certainly would find it really helpful if you know about the aforementioned problem.

mauricelanghinrichs commented 3 years ago

@pcjentsch thanks for your comment! This bias was only mentioned for earlystop=true, right? I think default is false and I hope that is fine to use (?). The DE method seems to be also faster for my problem, so I would be quite happy to (safely) continue using it.. :D

francescoalemanno commented 3 years ago

Thank you @pcjentsch and thank you @mauricelanghinrichs! As soon as I get some free time I would like to fix the state of this package and make a good release, However if you guys can get to it earlier I would very gladly accept a PR... Very sorry about the issues