GUDHI / gudhi-devel

The GUDHI library is a generic open source C++ library, with a Python interface, for Topological Data Analysis (TDA) and Higher Dimensional Geometry Understanding.
https://gudhi.inria.fr/
MIT License
246 stars 65 forks source link

Mix of random generators in tangential complex #983

Closed mglisse closed 9 months ago

mglisse commented 9 months ago

https://github.com/GUDHI/gudhi-devel/blob/c5ab9de0316ee88c28f81019aeb8e36bb5bd818f/src/Tangential_complex/include/gudhi/Tangential_complex.h#L1582-L1583

Here we have our own instance m_random_generator of CGAL::Random used to generate one number, but then Random_points_in_ball_d uses CGAL's instance (accessible as CGAL::get_default_random()). I think it would be more consistent to use the same for both, so either write CGAL::get_default_random().get_double here (and remove the useless m_random_generator), or pass m_random_generator as argument to the constructor of Random_points_in_ball_d.

(It is also a bit strange to generate a random radius and then a random point in a ball of that radius. It corresponds to a distribution on the ball of max radius that is heavier towards the center, which can make sense, but I don't see any comment about this, confirming that it is done on purpose, and explaining why this is the right formula.)