Exact Hamiltonian Monte Carlo for spherical Gaussian sampling #144
Conversation
| VT& Av, | ||
| int& facet_prev) const | ||
| { | ||
| return std::make_pair(0, 0); |
There was a problem hiding this comment.
Exceptions should be raised as in #142. The same applies for all classes where std::make_pair(0, 0) is called by actually no output is computed.
There was a problem hiding this comment.
@papachristoumarios Thank you for the comments!
This one changed according to #142
|
|
||
| // Exact HMC for sampling from the spherical Gaussian distribution | ||
|
|
||
| struct ExactHMCGaussianWalk |
There was a problem hiding this comment.
Naming consistency comments as in #142 .
| }; | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Too much whitespace here :)
| @@ -20,6 +21,9 @@ | |||
| #include "random_walks/uniform_john_walk.hpp" | |||
| #include "random_walks/uniform_vaidya_walk.hpp" | |||
| #include "random_walks/uniform_accelerated_billiard_walk.hpp" | |||
| #include "random_walks/exponential_hamiltonian_monte_carlo_walk.hpp" | |||
There was a problem hiding this comment.
Header naming consistency.
There was a problem hiding this comment.
Thanks. Done as mentioned in #142
include/convex_bodies/hpolytope.h
Outdated
| int m = num_of_hyperplanes(); | ||
| int facet = -1; | ||
|
|
||
| Ar.noalias() = A * r.getCoefficients(); |
There was a problem hiding this comment.
the two functions quadratic_positive_intersect differ only on how Ar is computed. Should you combine them?
There was a problem hiding this comment.
I implemented a new function get_positive_quadratic_root. Both oracles are using this function now.
include/convex_bodies/hpolytope.h
Outdated
| const double tol = 1e-10; | ||
| if (lamda1 * lamda2 < NT(0)) | ||
| { | ||
| if (previous_facet == current_facet) |
There was a problem hiding this comment.
this can be simplified
lamda = previous_facet == current_facet
? std::max(lamda1, lamda2) < NT(tol) ? std::min(lamda1, lamda2) : std::max(lamda1, lamda2)
: std::max(lamda1, lamda2);
There was a problem hiding this comment.
Also min max values can be precomputed (e.g. using std::minmax)
There was a problem hiding this comment.
Done, thanks for the code.
include/convex_bodies/hpolytope.h
Outdated
| sum_nom = Ar - b; | ||
| Av.noalias() = A * v.getCoefficients(); | ||
|
|
||
| NT* sum_nom_data = sum_nom.data(); |
There was a problem hiding this comment.
why are you using pointers (returned by data()) here?
There was a problem hiding this comment.
This is the standard approach in all boundary oracle - function in hpolytope.h.
include/convex_bodies/hpolytope.h
Outdated
| t1 = (2.0 * M_PI) / omega; | ||
| } | ||
|
|
||
| t2 = (-std::acos((*b_data) / C) - Phi) / omega; |
There was a problem hiding this comment.
std::acos((*b_data) / C) can be precomputed
| #ifndef RANDOM_WALKS_EXPONENTIAL_EXACT_HMC_WALK_HPP | ||
| #define RANDOM_WALKS_EXPONENTIAL_EXACT_HMC_WALK_HPP | ||
|
|
||
| #define IN_INSIDE_BODY_TOLLERANCE 1e-10 |
There was a problem hiding this comment.
shouldn't be named INSIDE_BODY_TOLLERANCE?
| for (unsigned int i=0; i<rnum; ++i) | ||
| { | ||
| success = walk.template apply(P, p, walk_length, rng); | ||
| if (!success) { |
There was a problem hiding this comment.
should an exception be thrown here instead?
| for (unsigned int i=0; i<rnum; ++i) | ||
| { | ||
| success = walk.template apply(P, p, walk_length, rng); | ||
| if (!success) { |
| if (nburns > 0) { | ||
| RandomPointGenerator::apply(P, p, nburns, walk_len, randPoints, | ||
| push_back_policy, rng); | ||
| randPoints.clear(); |
There was a problem hiding this comment.
do you need to store the random points here? It looks like a waste of space (and time).
| if (nburns > 0) { | ||
| RandomPointGenerator::apply(P, p, c, a, nburns, walk_len, randPoints, | ||
| push_back_policy, rng); | ||
| randPoints.clear(); |
| typename NT, | ||
| typename Point | ||
| > | ||
| void exponential_sampling(PointList &randPoints, |
This PR implements exact Hamiltonian Monte Carlo for spherical Gaussian sampling.
Algorithmic details:
-- The exact HMC for spherical Gaussian sampling exploits trigonometric trajectories in the interior of the polytope.
-- When the trajectory hits the boundary it is reflected.
Implementation details:
-- The sampler is implemented by a new structure.
-- The PR exploits the C++ interface for Gaussian sampling.
-- The sampler is integrated into the R interface of volesti.
This PR depends on #142