Add test for sphere and torus

Fix numerical approximations inconsistencies with dim fraction exponent when generating points as grid on torus
Add notes in doc regarding the torus versions use cases

6 files changed, 54 insertions, 5 deletions

diff --git a/src/common/include/gudhi/random_point_generators.h b/src/common/include/gudhi/random_point_generators.h
index 07e4f3da..25a7392d 100644
--- a/src/common/include/gudhi/random_point_generators.h
+++ b/src/common/include/gudhi/random_point_generators.h
@@ -227,7 +227,7 @@ std::vector<typename Kernel::Point_d> generate_points_on_torus_d(std::size_t num
   std::vector<Point> points;
   points.reserve(num_points);
   if (sample == "grid") {
-    std::size_t num_slices = (std::size_t)std::pow(num_points, 1. / dim);
+    std::size_t num_slices = (std::size_t)std::pow(num_points + .5, 1. / dim); // add .5 to avoid rounding down with numerical approximations
     generate_grid_points_on_torus_d(
                                        k, dim, num_slices, std::back_inserter(points), radius_noise_percentage);
   } else {
diff --git a/src/python/CMakeLists.txt b/src/python/CMakeLists.txt
index 8c46004a..f30dfe6d 100644
--- a/src/python/CMakeLists.txt
+++ b/src/python/CMakeLists.txt
@@ -443,6 +443,9 @@ if(PYTHONINTERP_FOUND)
       # Euclidean witness
       add_gudhi_py_test(test_euclidean_witness_complex)
 
+      # Datasets generators
+      add_gudhi_py_test(test_datasets_generators) # TODO separate full python datasets generators in another test file independant from CGAL ?
+
     endif (NOT CGAL_WITH_EIGEN3_VERSION VERSION_LESS 4.11.0)
 
     # Cubical
diff --git a/src/python/doc/datasets_generators.rst b/src/python/doc/datasets_generators.rst
index 2802eccd..e63dde82 100644
--- a/src/python/doc/datasets_generators.rst
+++ b/src/python/doc/datasets_generators.rst
@@ -60,6 +60,9 @@ Otherwise, if set to **'grid'**, the points are generated on a grid and would be
 
    ( [n\_samples^{1 \over {dim}}]^{dim}, 2*dim )
 
+
+**Note:** This version is recommended when the user wishes to use **'grid'** as sample type, or **'random'** with a relatively small number of samples (~ less than 150).
+
 Example
 """""""
 .. code-block:: python
@@ -81,6 +84,8 @@ The user should provide the number of points to be generated on the torus :code:
 The :code:`sample` argument is optional and is set to **'random'** by default.
 The other allowed value of sample type is **'grid'**.
 
+**Note:** This version is recommended when the user wishes to use **'random'** as sample type with a great number of samples and a low dimension.
+
 Example
 """""""
 .. code-block:: python
diff --git a/src/python/gudhi/datasets/generators/_points.cc b/src/python/gudhi/datasets/generators/_points.cc
index 6bbdf284..3d38ff90 100644
--- a/src/python/gudhi/datasets/generators/_points.cc
+++ b/src/python/gudhi/datasets/generators/_points.cc
@@ -48,6 +48,10 @@ py::array_t<double> generate_points_on_sphere(size_t n_samples, int ambient_dim,
 
 py::array_t<double> generate_points_on_torus(size_t n_samples, int dim, std::string sample) {
 
+    if ( (sample != "random") && (sample != "grid")) {
+        throw pybind11::value_error("This sample type is not supported");
+    }
+
     std::vector<typename Kern::Point_d> points_generated;
 
     {
diff --git a/src/python/gudhi/datasets/generators/points.py b/src/python/gudhi/datasets/generators/points.py
index 3870dea6..daada486 100644
--- a/src/python/gudhi/datasets/generators/points.py
+++ b/src/python/gudhi/datasets/generators/points.py
@@ -23,7 +23,7 @@ def _generate_random_points_on_torus(n_samples, dim):
 def _generate_grid_points_on_torus(n_samples, dim):
     
     # Generate points on a dim-torus as a grid
-    n_samples_grid = int(n_samples**(1./dim))
+    n_samples_grid = int((n_samples+.5)**(1./dim)) # add .5 to avoid rounding down with numerical approximations
     alpha = np.linspace(0, 2*np.pi, n_samples_grid, endpoint=False)
     
     array_points_inter = np.column_stack([np.cos(alpha), np.sin(alpha)])
@@ -45,12 +45,9 @@ def torus(n_samples, dim, sample='random'):
     """
     if sample == 'random':
         # Generate points randomly
-        print("Sample is random")
         return _generate_random_points_on_torus(n_samples, dim)
     elif sample == 'grid':
         # Generate points on a grid
-        print("Sample is grid")
         return _generate_grid_points_on_torus(n_samples, dim)
     else:
         raise ValueError("Sample type '{}' is not supported".format(sample))
-        return
diff --git a/src/python/test/test_datasets_generators.py b/src/python/test/test_datasets_generators.py
new file mode 100755
index 00000000..656c30ee
--- /dev/null
+++ b/src/python/test/test_datasets_generators.py
@@ -0,0 +1,40 @@
+""" This file is part of the Gudhi Library - https://gudhi.inria.fr/ - which is released under MIT.
+    See file LICENSE or go to https://gudhi.inria.fr/licensing/ for full license details.
+    Author(s):       Hind Montassif
+
+    Copyright (C) 2021 Inria
+
+    Modification(s):
+      - YYYY/MM Author: Description of the modification
+"""
+
+from gudhi.datasets.generators import points
+from gudhi.datasets.generators import _points
+
+import pytest
+
+def test_sphere():
+    assert _points.sphere(n_samples = 10, ambient_dim = 2, radius = 1., sample = 'random').shape == (10, 2)
+
+    with pytest.raises(ValueError):
+        _points.sphere(n_samples = 10, ambient_dim = 2, radius = 1., sample = 'other')
+
+def test_torus():
+    assert _points.torus(n_samples = 64, dim = 3, sample = 'random').shape == (64, 6)
+    assert _points.torus(n_samples = 64, dim = 3, sample = 'grid').shape == (64, 6)
+    
+    assert _points.torus(n_samples = 10, dim = 4, sample = 'random').shape == (10, 8)
+    assert _points.torus(n_samples = 10, dim = 4, sample = 'grid').shape == (1, 8)
+
+    with pytest.raises(ValueError):
+        _points.torus(n_samples = 10, dim = 4, sample = 'other')
+
+def test_torus_full_python():
+    assert points.torus(n_samples = 64, dim = 3, sample = 'random').shape == (64, 6)
+    assert points.torus(n_samples = 64, dim = 3, sample = 'grid').shape == (64, 6)
+    
+    assert points.torus(n_samples = 10, dim = 4, sample = 'random').shape == (10, 8)
+    assert points.torus(n_samples = 10, dim = 4, sample = 'grid').shape == (1, 8)
+
+    with pytest.raises(ValueError):
+        points.torus(n_samples = 10, dim = 4, sample = 'other')