Fix rand_phasepoint for non-Euclidean KE

[sethaxen]

rand_phasepoint should pass the point q to rand. i.e.

DynamicHMC.jl/src/hamiltonian.jl

Line 157 in 65f8a30

rand_phasepoint(rng::AbstractRNG, H, q) = phasepoint_in(H, q, rand(rng, H.κ))

should be

rand_phasepoint(rng::AbstractRNG, H, q) = phasepoint_in(H, q, rand(rng, H.κ, q))

This is consistent with the existing interface for rand used in

DynamicHMC.jl/src/hamiltonian.jl

Line 97 in 65f8a30

rand(rng::AbstractRNG, κ::GaussianKE, q = nothing) = κ.W * randn(rng, size(κ.W, 1))

and enables the user to use rand_phasepoint with user-defined non-EuclideanKEs.

[tpapp]

Thanks for reporting this. This code was (is being) reorganized in #44, but I will think about how to expose this.

Do you use or want to use non-Euclidean KE's with DynamicHMC?

[sethaxen]

Do you use or want to use non-Euclidean KE's with DynamicHMC?

Yes, I'm writing a package for HMC on manifolds that uses most of DynamicHMC's building blocks as mentioned in #43. Much of the interface here is sufficiently general that no changes are necessary, which is fantastic. I'll take a look at #44.

[tpapp]

I am very happy to support this in general, and take PRs against https://github.com/tpapp/DynamicHMC.jl/tree/tp/major-api-rewrite-2.0 (the branch where #44 is conducted).

The code should be much cleaner, feel free to ask questions about it here.

[tpapp]

Just a heads-up: everything in that PR is now merged, with subsequent improvements, and now I am preparing for a 2.0 release (#69).

This part of the API is deliberately not exported so we can always change/extend it without breaking anything if that's what you need. I am happy to take PRs about this.

[sethaxen]

Thanks! Sorry I hadn't replied yet. Shortly after my last comment, I realized that the simple expression of the generalized NUTS criterion on Riemannian manifolds in Betancourt's paper presumes one has a global coordinate system on the manifold. For general manifolds, even the sphere, this is not the case, and one will somehow need to parallel transport rho. This would at least mean that leapfrog would need to access rho, but it will likely be more involved. I haven't worked out how to do that yet in a way that's consistent with the theory, so this project is on hold for the moment.

Congrats on the great work! Looking forward to 2.0!