-
Notifications
You must be signed in to change notification settings - Fork 124
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
Order polytope improvements #319
Order polytope improvements #319
Conversation
lucaperju
commented
Jul 12, 2024
- Check that given DAG is acyclic when constructing a poset
- Generator for random Order polytopes
- Add possibility to generate Order Polytopes as Hpolytope class
* copy and replace lp_rlp.h * remove re-initialization of eta * update ubuntu version from 18 to 20 in R cran tests * minor improvements (explanatory comments) --------- Co-authored-by: Apostolos Chalkis <apostolos.chalkis@quantagonia.com>
* 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 (GeomScale#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>
…ce parameter, add define lp_solve guards to orderpolytope
Remove volesti4dingo branch
…_source Build only with lp-solve source code, no need for liblpsolve55.so files
* Enable c++17 support in tests and fix saxpy calls * Fix structure, code style and tests in autodiff
* correcting the includes to point the boost library directory * modifs to include only the include/ and external/ directories * modifs * Update CMakeLists.txt --------- Co-authored-by: Vissarion Fisikopoulos <fisikop@gmail.com> Co-authored-by: vfisikop <160006479+vfisikop@users.noreply.github.com>
Remove R interface
Refactor trigonometric_positive_intersect function for hpolytopes
update_position complexity improvement
* improve the implementation of maximum ellipsoid computation * minor fix in rounding unit-test * fix file copyrights * apply termination criterion after transformation in max ellipsoid rounding * resolve PR comments --------- Co-authored-by: Apostolos Chalkis <apostolos.chalkis@quantagonia.com>
…eomScale#310) * improve the implementation of maximum ellipsoid computation * minor fix in rounding unit-test * fix file copyrights * fix max_ball bug and create new unit test * fix max_ball bug and create new unit test * apply termination criterion after transformation in max ellipsoid rounding * imrpove skinny poly generator's interface * improve stopping criterion and seed setting * complete unit tests implementations * implement Newton method to compute the analytic center * minor changes * resolve PR comments * complete analytic center computation * resolve conflicts * minor changes * minor changes * minor changes * improve new unit tests * resolve PR comments --------- Co-authored-by: Apostolos Chalkis <apostolos.chalkis@quantagonia.com>
* Position Nudging after Position Update * Complexity improvements and Polytope Normalization * HPolytope Normalization Flag * Polytope normalization within Walk Constructor * Alias HPolytope Normalization for Nudging inside Gaussian HMC * Polytope Normalization in ComputeInner Ball Fixed * Polytope Normalization Style change * Nudge in function within Gaussian HMC, and restore Hpoly file * More efficient Nudge in Process
…ocessing routines (GeomScale#312) * generalize rounding loop * support sparse cholesky operator * complete sparse support in max_inscribed_ball * complete sparse support in preprocesing * add sparse tests * change main rounding function name * improve explaining comments * resolve PR comments * changing the dates in copyrights * use if constexpr instead of SNIFAE * update the examples to cpp17 * update to cpp17 order polytope example * fix templating in mat_computational_operators * fix templating errors and change header file to mat_computational_operators --------- Co-authored-by: Apostolos Chalkis <apostolos.chalkis@quantagonia.com>
* new function is_correlation_matrix * update example function call this example calls the is_correlation_matrix() function that is present in include/matrix_operations/EigenvaluesProblems.h * remove blank line * remove blank line * fix spaces, add function parameter
* generalize rounding loop * support sparse cholesky operator * complete sparse support in max_inscribed_ball * complete sparse support in preprocesing * add sparse tests * change main rounding function name * improve explaining comments * resolve PR comments * changing the dates in copyrights * use if constexpr instead of SNIFAE * update the examples to cpp17 * update to cpp17 order polytope example * fix templating in mat_computational_operators * fix templating errors and change header file to mat_computational_operators * first implementation of the volumetric barrier ellipsoid * add criterion for step_iter * restructure code that computes barriers' centers * remove unused comments * resolve PR comments * remove NT typename from max_step() --------- Co-authored-by: Apostolos Chalkis <apostolos.chalkis@quantagonia.com>
…lesti into order_polytope_improvements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks! Could you also add one or two unit tests to test the functionality?
// generates a Polytope from a poset | ||
/// @tparam Polytope Type of returned polytope | ||
template <class Polytope> | ||
Polytope get_orderpoly(Poset &poset) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
poset is not changed so why not passing it as a const reference?
OrderPolytope<Point> OP(poset); | ||
if constexpr (std::is_same< Polytope, OrderPolytope<Point> >::value ) { | ||
return OP; | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you assume that Polytope
type is either OrderPolytope
or HPolytope
but is could be of other type as well and this is not handled by this function. You could create an else statement with a static assert.
include/misc/poset.h
Outdated
@@ -25,6 +25,36 @@ class Poset { | |||
unsigned int n; // elements will be from 0 to n-1 | |||
RV order_relations; // pairs of form a <= b | |||
|
|||
static void sorted_list(std::vector<unsigned int> &res, const unsigned int &n, const RV &relations) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is more readable if you reorder parameters such that input parameters are first and output second, i.e., const unsigned int & n, const RV &relations, std::vector<unsigned int> &res
Polytope HP(OP.dimension(), OP.get_full_mat(), OP.get_vec()); | ||
return HP; | ||
} else { | ||
// TODO: implement functionality for more polytope types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A side note here: creating a V-polytope from an H-polytope representation of an order polytope is a hard problem (and cannot be approximated as volume computation) thus it is realistic to just remove that TODO ;)
…lesti into order_polytope_improvements
This reverts commit 0b97f7e.