Use propagate! instead of Libtask.consume

[devmotion]

This PR removes Libtask in the propagation of ParticleContainer. Instead it introduces a function propagate! that propagates all particles one step and returns true if all particles are propagated to the final time step and false otherwise (maybe one would rather want to use more explicit symbols instead of booleans?).

I always had the same impression as @mohamed82008 (see #1233) that in particular for the propagation of the particle container one does not have to use Libtask. However, since collect(::ParticleContainer) returns the particles IMO it would be surprising if iterating over a particle container would propagate it. Moreover, since it seems there are no states that have to be tracked by the container (I even removed n_consume and logE in the particle container), it felt more natural to add a method for propagation, similar to resample! for the resampling step, than using a dedicated iterator for ParticleContainer.

I didn't fully understand the existing implementation of the likelihood estimation (isn't the currently reported log evidence incorrect since logZ neglects the constant term -log(num_particles)?), so I used the algorithm in http://www.it.uu.se/research/systems_and_control/education/2019/smc/schedule/lecture9.pdf (pages 5, 6, and 7) as a template - basically, now the implementation for SMC and PG should perform exactly the same sequence of propagation and resampling steps as noted there using propagate! and resample!.

Aside from that, I'm slightly unhappy with the current implementation of SMC since it somehow misuses spl.state.average_logevidence to save the log likelihood estimate of all particles that is computed in sample_init!, then uses it in every iteration of step! in which the ParticleTransitions for every particle are created, and finally computes the "overall" log evidence in bundle_samples! by setting spl.state.average_logevidence to the mean of the log evidence values of these particles (i.e., the value that spl.state.average_logevidence was set to in sample_init!). It's also a bit annoying that for SMC an iteration keyword argument is needed (IMO that should be part of the state, i.e., transition and not be part of the generic algorithm in AbstractMCMC since it seems only/mostly SMC uses it).

[codecov]

Codecov Report

Merging #1237 into master will decrease coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1237      +/-   ##
==========================================
- Coverage   66.40%   66.25%   -0.16%     
==========================================
  Files          25       25              
  Lines        1289     1286       -3     
==========================================
- Hits          856      852       -4     
- Misses        433      434       +1     

Impacted Files	Coverage Δ
src/core/Core.jl	100.00% <ø> (ø)
src/inference/Inference.jl	79.79% <ø> (ø)
src/core/container.jl	93.02% <100.00%> (-1.72%)	⬇️
src/inference/AdvancedSMC.jl	98.57% <100.00%> (+0.06%)	⬆️

---

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 e75edf8...9623589. Read the comment docs.

[yebai]

I'm confused here. Shouldn't new weights be different from old weights, since no resampling is performed in propagate!?

[yebai]

OK, I forgot that there was no observation in the model. For the purpose of correctly testing propagate!, maybe consider adding some likelihood in the test?

[yebai]

Thanks, @devmotion. It's a good idea to remove the dependency on Libtask.comsume since it does not add any extra functionality except for recycling a function name. I think we can further make Particle's dependency on Libtask optional, by implementing an iterator interface mentioned by @mohamed82008 for CTask types. This way, the SMC implementation can support alternative approaches for resampling particles other than using Libtask.CTask, e.g. for less generic models that can be represented as Markov processes. In such models, we can either manually handle particle duplication during the resampling step, or consider something like ResumableFunctions.jl.

I didn't fully understand the existing implementation of the likelihood estimation (isn't the currently reported log evidence incorrect since logZ neglects the constant term -log(num_particles)?)

I think we had a step for subtracting the constant term in previous versions of the code, but I can't recall where that was, and when that was removed.

[devmotion]

I think we can further make Particle's dependency on Libtask optional, by implementing an iterator interface mentioned by @mohamed82008 for CTask types. This way, the SMC implementation can support alternative approaches for resampling particles other than using Libtask.CTask, e.g. for less generic models that can be represented as Markov processes. In such models, we can either manually handle particle duplication during the resampling step, or consider something like ResumableFunctions.jl.

I completely agree, probably it would be useful to implement and use an iterator interface for particles, so that one could use alternative particle implementations such as ordinary stateful iterators. IMO it's just not useful and unintuitive (due to the behaviour of collect) to implement an iterator interface for propagating a ParticleContainer. I thought the changes for Particles should better go into a separate PR (and as you said, one should first implement the interface for CTask).

[devmotion]

I'll update the test in a separate PR.