Switch design to immutable samplers

[devmotion]

This PR is a proposal to fix #31.

The current interface with sample_init!, step!, transitions, save!!, sample_end!, and bundle_samples is replaced with

• step(rng, model, sampler[, state; kwargs...]) that returns a 2-tuple of the next sample and state (similar to Base.iterate) and takes the state of the sampler as an argument in all but the initial step
• samples(sample, model, sampler[, N; kwargs...]) creates a container for the samples with initial sample sample and (potentially) predefined number of samples N
• save!!(samples, sample, iteration, model, sampler[, N; kwargs...]) saves the sample in the given iteration in the container of samples
• bundle_samples(samples, model, sampler, state, chain_type[; kwargs...]) creates the returned chain of type chain_type from the samples (the final state of the sampler can be saved as well if desired)

Additionally, the PR contains some improvements (which ended up in here but could be moved to separate PRs if desired) such as

• avoiding hangs if the parallel sampling errors by adding a try-finally block (as suggested a while ago on Slack)
• removing some unneeded copies in the parallel sampling with multiple processes.

[codecov]

Codecov Report

Merging #42 into master will decrease coverage by 0.28%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #42      +/-   ##
==========================================
- Coverage   97.82%   97.54%   -0.29%     
==========================================
  Files           6        6              
  Lines         138      122      -16     
==========================================
- Hits          135      119      -16     
  Misses          3        3              

Impacted Files	Coverage Δ
src/interface.jl	90.00% <100.00%> (-3.34%)	⬇️
src/sample.jl	100.00% <100.00%> (ø)
src/stepper.jl	71.42% <100.00%> (-3.58%)	⬇️
src/transducer.jl	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 509b5cf...f3fb684. Read the comment docs.

[cpfiffer]

Generally neat, thanks! I left a couple minor comments.

[devmotion]

I added a deprecation (and tests for it).

[cpfiffer]

Great work, thanks!

[devmotion]

Please don't merge this PR, even though it's approved.

I'd like to have working branches of DynamicPPL and Turing available locally before merging this PR (should be ready within the next days).

[cpfiffer]

Okay. Do you want to merge this into dev for now and then we can pull it into master when it's ready elsewhere?

[devmotion]

I don't think there's an advantage in creating a dev branch and moving it there? I'd just keep the PR open for now (also as a reminder that there's still some work to do).

[devmotion]

https://github.com/TuringLang/EllipticalSliceSampling.jl/compare/abstractmcmc2 shows the changes in EllipticalSliceSampling for this PR (also removed the ESS_mcmc method, by fully committing to sample we'll get the parallel sampling for free). IMO it's a lot cleaner now, the hack with transitions_init and transitions_save! is not needed anymore - the interface already provides a clear separation between samples and states.

[devmotion]

I'm wondering if we should actually just merge and release AbstractMCMC 2.0. There's no need to update all packages before it is released, with a new release I could just make a PR to EllipticalSliceSampling and run CI tests instead of relying on my local setup, and packages that are not compatible will be informed by CompatHelper about the new release but users will still only be able to use the version that was declared to be compatible.

[cpfiffer]

Yeah, I think it's fine. I believe most everything downstream has the correct Semver bounds.