Support intersection between an Hpolytope and an ellipsoid #287
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* initialize nuts sampler class * implement the burnin of nuts sampler * add tests and resolve bugs * implement e0 estimation in burnin of nuts * optimize leapfrog * integrate nuts into the R interface * document random walk in sample_points.cpp in R interface * fix burnin for the non-truncated case * resolve bugs in hmc and nuts pipelines * improve the preprocess in burin step of nuts * split large lines in sample_points.cpp * Add NUTS C++ example and update CMake (#29) * resolve PR comments * fix minor bug * fix compiler bug * fix error in building the C++ examples * resolve warnings in sample_points * fix lpsolve cran warning * fix cran warning on mac * improve lpsolve cmake for cran check * fix R warning in mac test * remove lpsolve header * resolve PR comments --------- Co-authored-by: Marios Papachristou <papachristoumarios@gmail.com> Co-authored-by: Apostolos Chalkis <apostolos.chalkis@quantagonia.com>
TolisChal
changed the title
TolisChal
changed the title
vissarion
reviewed
Nov 13, 2023
|
|
||
| /// This class represents a polytope intersected with an ellipsoid | ||
| /// \tparam Polytope Polytope Type | ||
| /// \tparam CEllipsoid Ellipsoid Type |
There was a problem hiding this comment.
why you call it CEllipsoid instead of just Ellipsoid?
| /// \tparam Polytope Polytope Type | ||
| /// \tparam CEllipsoid Ellipsoid Type | ||
| template <typename Polytope, typename CEllipsoid> | ||
| class EllipsoidIntersectPolytope { |
There was a problem hiding this comment.
Why not inherit from BallIntersectPolytope class? A lot of methods are the same. Also do we need this class at all or we can just use BallIntersectPolytope for both ball and ellipsoid intersections? The only difference I see is in the last 3 methods.
| *grad_data = ((params.a_vec.coeff(i) - NT(1)) / x.getCoefficients().coeff(i)); | ||
| grad_data++; | ||
| } | ||
| Point y(grad); |
There was a problem hiding this comment.
Why are you creating a VT and then copy it to a Point instead of creating a Point and assign the desired values directly into it?
| *grad_data = ((params.a_vec.coeff(i) - NT(1)) / x.getCoefficients().coeff(i)); | ||
| grad_data++; | ||
| } | ||
| Point y(grad); |
There was a problem hiding this comment.
same here, is there a copy here? Can we avoid it?
| template <typename NT> | ||
| static NT compute(EllipsoidIntersectPolytope<Polytope, Ellipsoid<Point>> &P) | ||
| { | ||
| return NT(1.5) * P.radius(); |
| parameters() : order(2), L(2), m(2), kappa(1) {}; | ||
|
|
||
| parameters(unsigned int order_) : | ||
| order(order), |
There was a problem hiding this comment.
you probably mean order(order_) here
| // Copyright (c) 2018-2023 Apostolos Chalkis | ||
| // Copyright (c) 2020-2023 Elias Tsigaridas | ||
|
|
||
| // Contributed and/or modified by Ioannis Iakovidis, as part of Google Summer of |
TolisChal
force-pushed
the
develop
branch
4 times, most recently
from
906e119 to
f1abc36
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR:
i) Provides a new C++ class that represents the intersection between an Hpolytope and an ellipsoid,
ii) Implements C++ examples on how to sample from the intersection between an Hpolytope and an ellipsoid,
iii) Exposes to R the new class using a new Rcpp module,
iv) Implements R example on how to sample from the intersection between an Hpolytope and an ellipsoid,
v) Provides the Dirichlet's distribution oracles in C++,
vi) Provides C++ examples to sample from the Dirichlet distribution,
vii) Implements R examples on how to sample from the Dirichlet distribution.