CRHMC Integration within Volume Cooling Gaussians #314

vgnecula · 2024-07-01T02:39:48Z

Integrated the CRHMC within the Volume Cooling Gaussians Algorithm.

Also, added an example file.

TolisChal · 2024-07-01T15:06:47Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+template <typename WalkType>
+struct update_delta
+{
+    template <typename NT>
+    static void apply(WalkType& walk, NT delta) {}
+};
+
+template <typename Polytope, typename RandomNumberGenerator>
+struct update_delta<GaussianBallWalk::Walk<Polytope, RandomNumberGenerator>>
+{
+    template <typename NT>
+    static void apply(GaussianBallWalk::Walk<Polytope, RandomNumberGenerator> walk,
+                      NT delta)
+    {
+        walk.update_delta(delta);
+    }
+};


Are they already defined in the volume_cooling_gaussians.hpp?

TolisChal · 2024-07-01T15:07:31Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+        update_delta<WalkType>
+                ::apply(walk, 4.0 * chebychev_radius
+                        / std::sqrt(std::max(NT(1.0), a_vals[it]) * NT(n)));


Do we need this here? Does this affect the crhmc parameterization?

TolisChal · 2024-07-01T15:08:30Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+        update_delta<WalkType>
+                ::apply(walk, 4.0 * radius
+                         / std::sqrt(std::max(NT(1.0), *avalsIt) * NT(n)));


Do we need this?

TolisChal · 2024-07-01T15:24:37Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+<
+    typename WalkTypePolicy = GaussianCDHRWalk,
+    typename RandomNumberGenerator = BoostRandomNumberGenerator<boost::mt11213b, double>,
+    typename Polytope,
+    int simdLen
+>
+double volume_cooling_gaussians(Polytope &Pin,
+                                 double const& error = 0.1,
+                                 unsigned int const& walk_length = 1)
+{
+    RandomNumberGenerator rng(Pin.dimension());
+    return volume_cooling_gaussians<WalkTypePolicy, Polytope, RandomNumberGenerator, simdLen>(Pin, rng, error, walk_length);
+}
+
+
+template
+<
+    typename WalkTypePolicy = GaussianCDHRWalk,
+    typename RandomNumberGenerator = BoostRandomNumberGenerator<boost::mt11213b, double>,
+    typename Polytope,
+    int simdLen
+>
+double volume_cooling_gaussians(Polytope &Pin,
+                                Cartesian<double>::Point const& interior_point,
+                                unsigned int const& walk_length = 1,
+                                double const& error = 0.1)
+{
+    RandomNumberGenerator rng(Pin.dimension());
+    Pin.set_interior_point(interior_point);
+
+    return volume_cooling_gaussians<WalkTypePolicy, Polytope, RandomNumberGenerator, simdLen>(Pin, rng, error, walk_length);
+}


What these calls are useful for? This implementation shouldn't be related to any other random walk.

TolisChal · 2024-07-03T16:34:43Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+
+// Compute the first variance a_0 for the starting gaussian
+template <typename Polytope, typename NT>
+void get_first_gaussian(Polytope& P,


Has this function differences from the get_first_gaussian in volume/volume_cooling_gaussian.hpp?

TolisChal · 2024-07-03T16:35:07Z

include/volume/volume_cooling_gaussians_crhmc.hpp

@@ -0,0 +1,550 @@
+#ifndef VOLUME_COOLING_GAUSSIANS_HPP


Plz change this to VOLUME_COOLING_GAUSSIANS_CRHMC_HPP

TolisChal · 2024-07-03T16:40:54Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+using NT = double;
+using Kernel = Cartesian<NT>;
+using Point = typename Kernel::Point;
+using Func = GaussianFunctor::FunctionFunctor<Point>;
+using Grad = GaussianFunctor::GradientFunctor<Point>;
+using Hess = GaussianFunctor::HessianFunctor<Point>;
+
+using NegativeLogprobFunctor = GaussianFunctor::FunctionFunctor<Point>; //Func
+using NegativeGradientFunctor = GaussianFunctor::GradientFunctor<Point>; //Grad
+using HessianFunctor = GaussianFunctor::HessianFunctor<Point>; //Hess
+
+using PolytopeType = HPolytope<Point>;
+using MT = PolytopeType::MT;
+using VT = Eigen::Matrix<NT, Eigen::Dynamic, 1>;
+using func_params = GaussianFunctor::parameters<NT, Point>;
+using RNG = BoostRandomNumberGenerator<boost::mt19937, NT>;
+
+//Param building -> see sampling_functions.cpp
+using Input = crhmc_input<MT, Point, Func, Grad, Hess>;
+using CrhmcProblem = crhmc_problem<Point, Input>;


We shouldn't define them here since this header file can be included in other files and cause redefinition errors. Plz use typedef ... for only what you need inside each function.

Also, you should use template parameters like NT, MT, VT, Polytope through the templated parameters in each function. For example, in volume_cooling_gaussians, you could define at the beggining,

typedef Polytope::NT NT;
typedef Polytope::MT MT;
....

You can check other templated volume functions and follow the same style.

vfisikop

Thanks! I have a few minor comments.

vfisikop · 2024-07-04T15:22:03Z

examples/crhmc_cooling_gaussians/CMakeLists.txt

+# VolEsti (volume computation and sampling library)
+# Copyright (c) 2012-2021 Vissarion Fisikopoulos
+# Copyright (c) 2018-2021 Apostolos Chalkis
+# Contributed and/or modified by Ioannis Iakovidis


please update those data

vfisikop · 2024-07-04T15:22:49Z

examples/crhmc_cooling_gaussians/volume_example.cpp

@@ -0,0 +1,49 @@
+#include "generators/known_polytope_generators.h"


add copyright headers here please

vfisikop · 2024-07-04T15:23:50Z

examples/crhmc_cooling_gaussians/volume_example.cpp

+typedef HPolytope<Point> HPOLYTOPE;
+
+void calculateAndVerifyVolume(HPOLYTOPE& polytope) {
+    int walk_len = 100;


does crhmc need such a large value for walk_length?

I found better results with a higher value. Of course, it is not a necessity, but compared to something like 10, it is surely performing better (at least based on my observations and the other current parameters). I think we can later do a better analysis on how this is performing. @TolisChal was also suggesting some modifications for N and W within the gaussian annealing.

In previous volume approximation methods best performance was obtained with walk_length=1.
However, we can check this later

Regarding N and W they should be much smaller than their default values, e.g., N=1200 and W=300

vfisikop · 2024-07-04T15:24:37Z

examples/crhmc_cooling_gaussians/volume_example.cpp

+    calculateAndVerifyVolume(birkhoff);
+
+    return 0;
+}


please add one empty line at the end of every file.

vfisikop · 2024-07-04T15:25:21Z

include/volume/volume_cooling_gaussians_crhmc.hpp

@@ -0,0 +1,448 @@
+#ifndef VOLUME_COOLING_GAUSSIANS_CRHMC_HPP


please add copyright headers here

vfisikop · 2024-07-04T15:33:54Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+        auto steps = totalSteps;
+
+
+        //create the crhmc problem for this variance


there is a potential of optimization here, i.e. to create the problem (and the preprocessing) only once and update each time with the new variance. Is the variance needed for the preprocessing or only for sampling?
Could we add a related TODO comment here?

vfisikop · 2024-07-04T15:35:24Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+
+////////////////////////////// Algorithms
+
+// Gaussian Anealling


we can remove this, we have it in the original case but you are creating those new functions especially for the second step, i.e. to sample from non-spherical gaussians.

vfisikop · 2024-07-04T15:36:13Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+    typename WalkTypePolicy = CRHMCWalk,
+    int simdLen = 8
+>
+double volume_cooling_gaussians_crhmc(Polytope& Pin,


could we keep the same name? It can be overloaded right?

Another question, what are the changes between this and the original volume_cooling_gaussians? If the changes are only the calls to get_first_gaussian and get_annealing_schedule and the type of the walk, is it possible to use the same function to avoid duplication of the code?

Actually not really. There are slight changes especially to the walk call, which is done within this function as well. For example walk instantiation, apply and after we apply the walk, in the current crhmc implementation the point is not automatically updated, so we need to use p = walk.getPoint();

vfisikop · 2024-07-04T15:36:58Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+
+    compute_annealing_schedule
+    <
+        WalkType,


maybe this it too much, could we just use 2 lines for the templates?

vfisikop · 2024-07-04T15:39:11Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+<
+    typename Polytope,
+    typename RandomNumberGenerator,
+    typename WalkTypePolicy = CRHMCWalk,


CRHMCWalk is the only case right, so this template could be removed.

TolisChal · 2024-07-17T02:37:29Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+    typedef CrhmcRandomPointGenerator<CRHMCWalkType> CRHMCRandomPointGenerator;
+
+    CRHMCRandomPointGenerator::apply(problem, p, N, walk_length, randPoints,
+                                push_back_policy, rng, g, f, parameters, crhmc_walk, simdLen, raw_output);


plz fix indentation here

TolisChal · 2024-07-17T02:38:42Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+    //const NT maxNT = std::numeric_limits<NT>::max();//1.79769e+308;
+    //const NT minNT = std::numeric_limits<NT>::min();//-1.79769e+308;


Shall we delete those two lines?

TolisChal · 2024-07-17T02:38:54Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+    unsigned int n = P.dimension();
+    unsigned int m = P.num_of_hyperplanes();
+    gaussian_annealing_parameters<NT> parameters(P.dimension());
+    //RandomNumberGenerator rng(n);


Shall we delete those this line?

TolisChal · 2024-07-17T02:41:52Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+{
+
+    typedef typename Polytope::PointType Point;
+


plz delete this blank line

TolisChal · 2024-07-17T02:42:09Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+                k = k / 2;
+            }
+            done = true;
+        } else {


} else
{ ...

TolisChal · 2024-07-19T16:07:06Z

include/volume/volume_cooling_gaussians_crhmc.hpp

+    typedef CrhmcRandomPointGenerator<CRHMCWalkType> CRHMCRandomPointGenerator;
+
+    CRHMCRandomPointGenerator::apply(problem, p, N, walk_length, randPoints,
+                                    push_back_policy, rng, g, f, parameters, crhmc_walk, simdLen, raw_output);


plz fix indentation here

vgnecula marked this pull request as ready for review July 1, 2024 02:39

vgnecula changed the title ~~CRHMC Integratiojn within Volume Cooling Gaussians~~ CRHMC Integration within Volume Cooling Gaussians Jul 1, 2024

TolisChal reviewed Jul 1, 2024

View reviewed changes

TolisChal reviewed Jul 3, 2024

View reviewed changes

vfisikop reviewed Jul 4, 2024

View reviewed changes

TolisChal force-pushed the develop branch 4 times, most recently from 906e119 to f1abc36 Compare July 7, 2024 02:25

vfisikop approved these changes Jul 16, 2024

View reviewed changes

TolisChal reviewed Jul 17, 2024

View reviewed changes

include/volume/volume_cooling_gaussians_crhmc.hpp Outdated

{

typedef typename Polytope::PointType Point;

Copy link

Member

TolisChal Jul 17, 2024

Choose a reason for hiding this comment

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

plz delete this blank line

TolisChal reviewed Jul 17, 2024

View reviewed changes

include/volume/volume_cooling_gaussians_crhmc.hpp Outdated

k = k / 2;

}

done = true;

} else {

Copy link

Member

TolisChal Jul 17, 2024

Choose a reason for hiding this comment

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

} else
{ ...

TolisChal force-pushed the develop branch from 992d203 to 7a0be2d Compare July 17, 2024 03:38

vgnecula closed this Jul 18, 2024

vgnecula force-pushed the crhmc_cooling_gaussians branch from eaca7fc to 7a0be2d Compare July 18, 2024 14:40

Rebased code with develop

1addf5e

vgnecula reopened this Jul 18, 2024

vgnecula added 2 commits July 18, 2024 12:08

Fix identation

2b88ecd

Fixed crhmc walk non-template call problem

2920e9c

TolisChal reviewed Jul 19, 2024

View reviewed changes

TolisChal approved these changes Jul 19, 2024

View reviewed changes

TolisChal merged commit 3325d6a into GeomScale:develop Jul 19, 2024
25 of 27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CRHMC Integration within Volume Cooling Gaussians #314

CRHMC Integration within Volume Cooling Gaussians #314

vgnecula commented Jul 1, 2024

TolisChal Jul 1, 2024

TolisChal Jul 1, 2024

TolisChal Jul 1, 2024

TolisChal Jul 1, 2024

TolisChal Jul 3, 2024

TolisChal Jul 3, 2024

TolisChal Jul 3, 2024

vfisikop left a comment

vfisikop Jul 4, 2024

vfisikop Jul 4, 2024

vfisikop Jul 4, 2024

vgnecula Jul 4, 2024

TolisChal Jul 17, 2024

TolisChal Jul 17, 2024

vfisikop Jul 4, 2024

vfisikop Jul 4, 2024

vfisikop Jul 4, 2024

vfisikop Jul 4, 2024

vfisikop Jul 4, 2024

vfisikop Jul 4, 2024

vgnecula Jul 4, 2024

vfisikop Jul 4, 2024

vfisikop Jul 4, 2024

TolisChal Jul 17, 2024

TolisChal Jul 17, 2024

TolisChal Jul 17, 2024

TolisChal Jul 17, 2024

TolisChal Jul 17, 2024

TolisChal Jul 19, 2024

		@@ -0,0 +1,49 @@
		#include "generators/known_polytope_generators.h"

		auto steps = totalSteps;


		//create the crhmc problem for this variance


		////////////////////////////// Algorithms

		// Gaussian Anealling

		//const NT maxNT = std::numeric_limits<NT>::max();//1.79769e+308;
		//const NT minNT = std::numeric_limits<NT>::min();//-1.79769e+308;

CRHMC Integration within Volume Cooling Gaussians #314

CRHMC Integration within Volume Cooling Gaussians #314

Conversation

vgnecula commented Jul 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vfisikop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment