search

TuringLang / AdvancedHMC.jl

Robust, modular and efficient implementation of advanced Hamiltonian Monte Carlo algorithms
https://turinglang.org/AdvancedHMC.jl/
MIT License
228 stars 39 forks source link

`AbstractMCMC.step` interface broken #350

Open devmotion opened 10 months ago

devmotion commented 10 months ago

https://github.com/TuringLang/AdvancedHMC.jl/pull/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 https://github.com/TuringLang/AdvancedHMC.jl/blob/04ce92ab167df3c2a309e845911129d3a1d43aa7/src/abstractmcmc.jl#L161 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 commented 10 months ago

Just remembered https://github.com/TuringLang/AbstractMCMC.jl/pull/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 commented 9 months ago

@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 commented 9 months ago

https://github.com/TuringLang/AbstractMCMC.jl/pull/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.