AbstractMCMC.step interface broken

[devmotion]

#332 broke the AbstractMCMC.step interface: It's not possible anymore to initialize the sampler, obtain the initial state, and then continue sampling with AbstractMCMC.step.

The problem is

AdvancedHMC.jl/src/abstractmcmc.jl

Line 161 in 04ce92a

n_adapts = kwargs[:n_adapts]

It assumes that nadapts is provided as a keyword argument to the step function by the user. This is not part of the AbstractMCMC.step interface requirements. If AdvancedHMC wants to use such a keyword argument in the function body it should be possible to omit it and have a default value. I think that's still a bad idea though since then users/developers have to know about how AdvancedHMC internally (ab)uses keyword arguments to be able to pass around this information in the same way. IMO it would be better to put this into the state of the sampler if it's not intended to be fixed a priori when constructing the sampler.

[devmotion]

Just remembered TuringLang/AbstractMCMC.jl#124.

I still think this should be fixed in AdvancedHMC rather than AbstractMCMC - we can't add all downstream options to AbstractMCMC and it means that custom implementations based on step (as in my use case) will always use a default value of 0, if they don't know about and reimplement the logic in AdvancedHMC.

[yebai]

@devmotion @torfjelde @JaimeRZP do we have a default mechanism for users to pass warmup strategy somewhere? If not, it would be good to introduce an AbstractWarmUp type in AbstractMCMC so we have a consistent interface for passing warmup strategies in AbstractMCMC.step functions.

[devmotion]

TuringLang/AbstractMCMC.jl#117 will allow samplers to specify behaviour of warm-up steps. AdvancedHMC would specialize step_warmup, possibly depending on a warm-up algorithm instance stored in the sampler.

[devmotion]

I'll re-open because I think it would be better for interoperability with other packages to avoid "magic" keyword arguments.