Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ellipsoid walks: Dikin, John and Vaidya walk. #39

Closed
wants to merge 3 commits into from

Conversation

TolisChal
Copy link
Member

This PR contains three geometric random walks for uniform sampling from a convex polytope when the latter is given in H-representation:

  • Dikin walk
  • John walk
  • Vaidya walk

We add the implementations from https://github.com/rzrsk/vaidya-walk and we make changes in order to be used efficiently from volesti package.

@@ -0,0 +1,93 @@
// Code from https://github.com/rzrsk/vaidya-walk
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add
// Related publication: Chen et al. - "Fast MCMC Sampling Algorithms on Polytopes", Journal of Machine Learning Research 19 (2018) 1-86.
similarly to all files

Also please add the info and copyright notes as in all the files of the library

@@ -101,7 +107,8 @@ Rcpp::NumericMatrix sample_points(Rcpp::Nullable<Rcpp::Reference> P = R_NilValue

int type, dim, numpoints;
NT radius = 1.0, delta = -1.0;
bool set_mean_point = false, cdhr = true, rdhr = false, ball_walk = false, gaussian = false;
bool set_mean_point = false, cdhr = false, rdhr = false, ball_walk = false, gaussian = false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why having all those bool vars, one per walk type and not just one enum that takes the value related to the walk type, e.g. enum Walk { RDHnR, CDHnR, Ball, Billiard, Vaidya, John, Dikin, ... };

@vissarion vissarion mentioned this pull request Sep 11, 2019
@TolisChal TolisChal closed this Sep 12, 2019
@TolisChal TolisChal deleted the ellips_walks branch September 12, 2019 10:34
@TolisChal TolisChal restored the ellips_walks branch September 12, 2019 11:01
@TolisChal TolisChal reopened this Sep 12, 2019
@vissarion vissarion force-pushed the develop branch 11 times, most recently from e4ac563 to ca784f5 Compare February 27, 2020 15:07
@vissarion
Copy link
Member

This is re-worked here #88

@vissarion vissarion closed this Sep 8, 2020
@TolisChal TolisChal deleted the ellips_walks branch September 27, 2020 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants