Some improvements to sampling about soss.jl HOT 3 CLOSED

Hi @sethaxen ,

I fixed some bugs in #68 (thanks again for initiating this), and I dropped the need to specify N as a keyword argument. So as it stands, you can call particles(m) or particles(m,N::Int). Do you see a compelling reason to have this as a keyword argument? If not, do you still see a benefit from splitting the function, rather than just having multiple methods? Do you have results other than Particles in mind?

I really like the idea of overloading mcmc_keep_warmup. And since we own JointDistribution, there's no concern for type piracy.

For your third point, I had played a bit with having just a single sample function taking a method argument, that could be e.g. dynamicHMC or advancedHMC. I don't think it was necessarily a bad approach, but it did make the internals a little more complex and I didn't see much benefit at the time. Happy to come back to this (or something similar) if you think it would make a big difference.

from soss.jl.

Do you see a compelling reason to have this as a keyword argument?

No, this is fine.

If not, do you still see a benefit from splitting the function, rather than just having multiple methods? Do you have results other than Particles in mind?

For your third point, I had played a bit with having just a single sample function taking a method argument, that could be e.g. dynamicHMC or advancedHMC.

I've not thought about this very much, but I like the idea of having a generic function like sample. I was thinking it would be handy to have a preferred order of sampling algorithms. e.g. exact sampling if possible, dynamic HMC if all continuous, Gibbs sampler with dynamic HMC and MH otherwise. I would consider this an advanced feature though.

I'm mainly concerned about cases when Particles output maybe isn't appropriate. e.g. what if a variable is a custom type that represents a point on a manifold? If the variable is just an Array, you'll get Array{Particles}, where each Particles corresponds to an element of the array. But if you used a custom type such as a Manifolds.MPoint, you'd probably want a Particles{<:Manifolds.MPoint}. If that works and everything will behave correctly, then maybe these concerns are irrelevant.

I really like the idea of overloading mcmc_keep_warmup. And since we own JointDistribution, there's no concern for type piracy.

I think this is a good idea either way, and I'll submit a PR for this and overloading the necessary function(s) for AdvancedHMC.sample as well.

It would also be nice to have something like Turing's psample for sampling chains in parallel.

from soss.jl.

After taking a stab at this, one problem with overloading mcmc_keep_warmup to create the TransformedLogDensity is that the same TransformedLogDensity is needed to transform the samples, and it won't be available to us if we create it in the method. I'll think some more tomorrow if there's a better abstraction.

from soss.jl.