A lot of the code in AHMC "unnecessarily" requires a Hamiltonian, i.e. a struct containing metric, ℓπ, and ∂ℓπ∂θ, to be passed around. This is a bit awkward for the following reasons:
Only the metric actually changes during sampling. ℓπ and ∂ℓπ∂θ are of course fixed. Hence it's a bit more natural to simply keep track of the metric rather than the full Hamiltonian object.
Even when using the AbstractMCMC-interface, we unnecessarily need to construct the Hamiltonian for to access certain functionality, e.g. find_good_stepsize.
IMO it's a more composable if we instead pass around metric and DifferentiableDensityModel (which contains the latter two), or, in more generality, a AbstractMCMC.AbstractModel, and the Hamiltonian should be constructed when needed e.g. at the call-site of AdvancedHMC.step or maybe even add a default impl for DifferentiableDensityModel
function step(int::AbstractIntegrator, model::DifferentiableDensityModel, metric::AbstractMetric, z::P, n_steps::Int=1; kwargs...)
h = Hamiltonian(model, metric)
return step(int, h, z, n_steps; kwargs...)
end
This would allow for futher extensions down the road, e.g. maybe it's useful to allow some other AbstractModel to be used, etc. But also it would mean that we can share more code between the impl of AbstractMCMC.jl-interface and the "standard" interface of AHMC.
A lot of the code in AHMC "unnecessarily" requires a
Hamiltonian, i.e. a struct containingmetric,ℓπ, and∂ℓπ∂θ, to be passed around. This is a bit awkward for the following reasons:metricactually changes during sampling.ℓπand∂ℓπ∂θare of course fixed. Hence it's a bit more natural to simply keep track of themetricrather than the fullHamiltonianobject.Hamiltonianfor to access certain functionality, e.g.find_good_stepsize.IMO it's a more composable if we instead pass around
metricandDifferentiableDensityModel(which contains the latter two), or, in more generality, aAbstractMCMC.AbstractModel, and theHamiltonianshould be constructed when needed e.g. at the call-site ofAdvancedHMC.stepor maybe even add a default impl forDifferentiableDensityModelThis would allow for futher extensions down the road, e.g. maybe it's useful to allow some other
AbstractModelto be used, etc. But also it would mean that we can share more code between the impl of AbstractMCMC.jl-interface and the "standard" interface of AHMC.