Update to AbstractMCMC 2

[devmotion]

This is a quite rough and initial draft of how to update DynamicPPL + Turing to AbstractMCMC 2 (tests don't pass currently). The update requires some major changes since sample_init! and sample_end! were removed. Some changes might have to be reverted or moved to other PRs, while others might be missing.

[devmotion]

I just rechecked, it seems most tests pass, mainly/only Gibbs is broken.

[devmotion]

Gibbs works as well now, but I'm not satisfied with the current implementation and the DynamicPPL-specific interface for the steps. It seems a bit too complicated, and in particular Gibbs + HMC have some weird initial steps.

I won't be able to work on this for the next two weeks but it's in a working state now (apart from the PDMats error tests passed locally).

[devmotion]

bors try

[bors]

try

Build failed:

• test (1, ubuntu-latest, x64)

[devmotion]

bors try

[bors]

try

Build failed:

• test (1, ubuntu-latest, x86)

[devmotion]

bors try

[bors]

try

Build failed:

• test (1, ubuntu-latest, x64)

[devmotion]

bors try

[bors]

try

Build failed:

• test (1.3, ubuntu-latest, x64)

[devmotion]

bors try

[bors]

try

Build failed:

• test (1, ubuntu-latest, x64)

[devmotion]

bors try

[bors]

try

Build failed:

• test (1, ubuntu-latest, x64)

[torfjelde]

Curious if there's anything I can do to help with this process!
I have some stuff I'd like to do in Turing which will be sooo much easier in the new (much nicer) interface you implemented:)

[devmotion]

Sorry, I was quite busy the last weeks (had some deadlines last week), but hopefully this can be merged soon 🙂 It seems to work mostly, apart from some missing N in the definition of the HMC algorithm (IIRC the number of iterations has to be saved somewhere if we want to keep the same logic since it is not provided by AbstractMCMC anymore in the step where it was used before). As a first step, I updated the Turing folder (#173). If it is merged and dev is updated, I will rebase this PR such that it can be reviewed (after fixing the HMC issue).

[cpfiffer]

Do you want a second pass on this one?

[devmotion]

I also updated the corresponding PR to Turing: TuringLang/Turing.jl#1428 Tests pass locally.

[cpfiffer]

I'll conditionally approve -- I left two comments but they are extremely minor. Well done!

[cpfiffer]

Okay, I'm good to merge this then.

[devmotion]

@yebai @torfjelde @cpfiffer My plan would be to squash and merge this into the master branch, release a new breaking version 0.10 of DynamicPPL, and then update Turing by merging the corresponding PR over there (when tests pass - locally they do). Keeping this change in a separate branch (e.g. dev) would just delay the propagation of the changes to Turing which doesn't seem useful - but maybe you disagree? What's your opinion on this?

[cpfiffer]

I say merge it in. No point in putting this into a dev branch, I think.

[torfjelde]

I'm also fine with merging into master for this. I'll complete my review this weekend (sorry for the delay; been a proper week) if that's fine, though from a quick look I don't see anything that would make me reject.

[yebai]

@yebai @torfjelde @cpfiffer My plan would be to squash and merge this into the master branch, release a new breaking version 0.10 of DynamicPPL, and then update Turing by merging the corresponding PR over there (when tests pass - locally they do). Keeping this change in a separate branch (e.g. dev) would just delay the propagation of the changes to Turing which doesn't seem useful - but maybe you disagree? What's your opinion on this?

The plan sounds good to me!

[torfjelde]

I still have some unresolved comments but nothing breaking (one code suggestion which is simply to make it a bit nicer to read), so I'm going to to approve now 👍

Awesome stuff @devmotion !

[devmotion]

I propose a slight deviation from the plan above: maybe we can merge #193 first, so also users with the current Turing + AbstractMCMC setup can install Distributions 0.24?

[cpfiffer]

Sure, works for me.

[devmotion]

I noticed that it is not completely straightforward to update Turing to Distributions 0.24 without this PR: #193 (comment)

[devmotion]

Can someone review (and hopefully approve) #195 so we can run the tests properly when merging this PR?

[devmotion]

Are you fine with this PR and approve it @torfjelde? I lost track a bit which things you still would like to be addressed 🙂

[torfjelde]

LGTM! :)

[devmotion]

Perfect 🙂

bors r+