From 68031184fb94cf19c8b3c6f0de122db447693847 Mon Sep 17 00:00:00 2001 From: Hind-M Date: Tue, 29 Jun 2021 11:48:28 +0200 Subject: Fix issue #502: check homology_coeff_field primality before computing persistence --- src/python/include/Persistent_cohomology_interface.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'src') diff --git a/src/python/include/Persistent_cohomology_interface.h b/src/python/include/Persistent_cohomology_interface.h index e5a3dfba..6877f190 100644 --- a/src/python/include/Persistent_cohomology_interface.h +++ b/src/python/include/Persistent_cohomology_interface.h @@ -43,6 +43,21 @@ persistent_cohomology::Persistent_cohomology 1; + if ((n % 2 == 0) || (n % 3 == 0)) + return false; + int i = 5; + while (i*i <= n) { + if ((n % i == 0) || (n % (i + 2) == 0)) + return false; + i += 6; + } + return true; + } + public: Persistent_cohomology_interface(FilteredComplex* stptr, bool persistence_dim_max=false) : Base(*stptr, persistence_dim_max), @@ -50,6 +65,11 @@ persistent_cohomology::Persistent_cohomology Date: Thu, 1 Jul 2021 15:56:11 +0200 Subject: Move primality test to Field_Zp::init Throw exception when not prime Add tests --- src/Persistent_cohomology/example/CMakeLists.txt | 2 +- .../include/gudhi/Persistent_cohomology/Field_Zp.h | 19 ++++++++++++++++++- .../test/persistent_cohomology_unit_test.cpp | 22 +++++++++++++++++++++- src/python/gudhi/cubical_complex.pyx | 2 +- src/python/gudhi/periodic_cubical_complex.pyx | 2 +- src/python/gudhi/simplex_tree.pxd | 2 +- .../include/Persistent_cohomology_interface.h | 20 -------------------- 7 files changed, 43 insertions(+), 26 deletions(-) (limited to 'src') diff --git a/src/Persistent_cohomology/example/CMakeLists.txt b/src/Persistent_cohomology/example/CMakeLists.txt index c68c6524..3e7e9369 100644 --- a/src/Persistent_cohomology/example/CMakeLists.txt +++ b/src/Persistent_cohomology/example/CMakeLists.txt @@ -11,7 +11,7 @@ if (TBB_FOUND) target_link_libraries(persistence_from_simple_simplex_tree ${TBB_LIBRARIES}) endif() add_test(NAME Persistent_cohomology_example_from_simple_simplex_tree COMMAND $ - "1" "0") + "2" "0") if(TARGET Boost::program_options) add_executable(rips_persistence_step_by_step rips_persistence_step_by_step.cpp) diff --git a/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h b/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h index 0673625c..4bfd95c0 100644 --- a/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h +++ b/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h @@ -13,6 +13,8 @@ #include #include +#include +#include namespace Gudhi { @@ -34,15 +36,30 @@ class Field_Zp { void init(int charac) { assert(charac > 0); // division by zero + non negative values + Prime = charac; + + // Check for primality + if ((Prime == 0) || (Prime == 1) || ((Prime > 3) && ((Prime % 2 == 0) || (Prime % 3 == 0)))) + throw std::invalid_argument("homology_coeff_field must be a prime number"); + inverse_.clear(); inverse_.reserve(charac); inverse_.push_back(0); for (int i = 1; i < Prime; ++i) { int inv = 1; - while (((inv * i) % Prime) != 1) + int mult = inv * i; + while ( (mult % Prime) != 1) { ++inv; + if(mult == Prime) + throw std::invalid_argument("homology_coeff_field must be a prime number"); + mult = inv * i; + } inverse_.push_back(inv); + if ( (i <= std::sqrt(Prime)) && (((i-5)%6) == 0) ) { + if ((Prime % i == 0) || (Prime % (i + 2) == 0)) + throw std::invalid_argument("homology_coeff_field must be a prime number"); + } } } diff --git a/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp b/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp index fe3f8517..9559b842 100644 --- a/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp +++ b/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp @@ -146,9 +146,14 @@ void test_rips_persistence_in_dimension(int dimension) { std::clog << "str_rips_persistence=" << str_rips_persistence << std::endl; } +BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_0 ) +{ + BOOST_CHECK_THROW(test_rips_persistence_in_dimension(0), std::invalid_argument); +} + BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_1 ) { - test_rips_persistence_in_dimension(1); + BOOST_CHECK_THROW(test_rips_persistence_in_dimension(1), std::invalid_argument); } BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_2 ) @@ -161,11 +166,26 @@ BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_3 ) test_rips_persistence_in_dimension(3); } +BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_4 ) +{ + BOOST_CHECK_THROW(test_rips_persistence_in_dimension(4), std::invalid_argument); +} + BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_5 ) { test_rips_persistence_in_dimension(5); } +BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_11 ) +{ + test_rips_persistence_in_dimension(11); +} + +BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_13 ) +{ + test_rips_persistence_in_dimension(13); +} + // TODO(VR): not working from 6 // std::string str_rips_persistence = test_rips_persistence(6, 0); // TODO(VR): division by zero diff --git a/src/python/gudhi/cubical_complex.pyx b/src/python/gudhi/cubical_complex.pyx index 28fbe3af..adc40499 100644 --- a/src/python/gudhi/cubical_complex.pyx +++ b/src/python/gudhi/cubical_complex.pyx @@ -35,7 +35,7 @@ cdef extern from "Cubical_complex_interface.h" namespace "Gudhi": cdef extern from "Persistent_cohomology_interface.h" namespace "Gudhi": cdef cppclass Cubical_complex_persistence_interface "Gudhi::Persistent_cohomology_interface>": Cubical_complex_persistence_interface(Bitmap_cubical_complex_base_interface * st, bool persistence_dim_max) nogil - void compute_persistence(int homology_coeff_field, double min_persistence) nogil + void compute_persistence(int homology_coeff_field, double min_persistence) nogil except+ vector[pair[int, pair[double, double]]] get_persistence() nogil vector[vector[int]] cofaces_of_cubical_persistence_pairs() nogil vector[int] betti_numbers() nogil diff --git a/src/python/gudhi/periodic_cubical_complex.pyx b/src/python/gudhi/periodic_cubical_complex.pyx index d353d2af..0eaa5867 100644 --- a/src/python/gudhi/periodic_cubical_complex.pyx +++ b/src/python/gudhi/periodic_cubical_complex.pyx @@ -32,7 +32,7 @@ cdef extern from "Cubical_complex_interface.h" namespace "Gudhi": cdef extern from "Persistent_cohomology_interface.h" namespace "Gudhi": cdef cppclass Periodic_cubical_complex_persistence_interface "Gudhi::Persistent_cohomology_interface>>": Periodic_cubical_complex_persistence_interface(Periodic_cubical_complex_base_interface * st, bool persistence_dim_max) nogil - void compute_persistence(int homology_coeff_field, double min_persistence) nogil + void compute_persistence(int homology_coeff_field, double min_persistence) nogil except + vector[pair[int, pair[double, double]]] get_persistence() nogil vector[vector[int]] cofaces_of_cubical_persistence_pairs() nogil vector[int] betti_numbers() nogil diff --git a/src/python/gudhi/simplex_tree.pxd b/src/python/gudhi/simplex_tree.pxd index 3b8ea4f9..006a24ed 100644 --- a/src/python/gudhi/simplex_tree.pxd +++ b/src/python/gudhi/simplex_tree.pxd @@ -78,7 +78,7 @@ cdef extern from "Simplex_tree_interface.h" namespace "Gudhi": cdef extern from "Persistent_cohomology_interface.h" namespace "Gudhi": cdef cppclass Simplex_tree_persistence_interface "Gudhi::Persistent_cohomology_interface>": Simplex_tree_persistence_interface(Simplex_tree_interface_full_featured * st, bool persistence_dim_max) nogil - void compute_persistence(int homology_coeff_field, double min_persistence) nogil + void compute_persistence(int homology_coeff_field, double min_persistence) nogil except + vector[pair[int, pair[double, double]]] get_persistence() nogil vector[int] betti_numbers() nogil vector[int] persistent_betti_numbers(double from_value, double to_value) nogil diff --git a/src/python/include/Persistent_cohomology_interface.h b/src/python/include/Persistent_cohomology_interface.h index 6877f190..e5a3dfba 100644 --- a/src/python/include/Persistent_cohomology_interface.h +++ b/src/python/include/Persistent_cohomology_interface.h @@ -43,21 +43,6 @@ persistent_cohomology::Persistent_cohomology 1; - if ((n % 2 == 0) || (n % 3 == 0)) - return false; - int i = 5; - while (i*i <= n) { - if ((n % i == 0) || (n % (i + 2) == 0)) - return false; - i += 6; - } - return true; - } - public: Persistent_cohomology_interface(FilteredComplex* stptr, bool persistence_dim_max=false) : Base(*stptr, persistence_dim_max), @@ -65,11 +50,6 @@ persistent_cohomology::Persistent_cohomology Date: Mon, 12 Jul 2021 11:25:57 +0200 Subject: Limit homology_coeff_field value to max allowed Add test with first prime outside the allowed range --- .../include/gudhi/Persistent_cohomology/Field_Zp.h | 7 +++++-- .../test/persistent_cohomology_unit_test.cpp | 8 ++++---- src/python/gudhi/simplex_tree.pyx | 6 +++--- 3 files changed, 12 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h b/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h index 4bfd95c0..7ecc9a80 100644 --- a/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h +++ b/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h @@ -14,7 +14,6 @@ #include #include #include -#include namespace Gudhi { @@ -39,6 +38,10 @@ class Field_Zp { Prime = charac; + // Check that the provided prime is less than the maximum allowed as int and calculation below : 46337 ; i.e (max_prime-1)**2 <= INT_MAX + if(Prime > 46337) + throw std::invalid_argument("Maximum homology_coeff_field allowed value is 46337"); + // Check for primality if ((Prime == 0) || (Prime == 1) || ((Prime > 3) && ((Prime % 2 == 0) || (Prime % 3 == 0)))) throw std::invalid_argument("homology_coeff_field must be a prime number"); @@ -56,7 +59,7 @@ class Field_Zp { mult = inv * i; } inverse_.push_back(inv); - if ( (i <= std::sqrt(Prime)) && (((i-5)%6) == 0) ) { + if ( (i*i <= Prime) && (((i-5)%6) == 0) ) { if ((Prime % i == 0) || (Prime % (i + 2) == 0)) throw std::invalid_argument("homology_coeff_field must be a prime number"); } diff --git a/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp b/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp index 9559b842..35bb5988 100644 --- a/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp +++ b/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp @@ -186,10 +186,10 @@ BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_13 ) test_rips_persistence_in_dimension(13); } -// TODO(VR): not working from 6 -// std::string str_rips_persistence = test_rips_persistence(6, 0); -// TODO(VR): division by zero -// std::string str_rips_persistence = test_rips_persistence(0, 0); +BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_46349 ) +{ + BOOST_CHECK_THROW(test_rips_persistence_in_dimension(46349), std::invalid_argument); +} /** SimplexTree minimal options to test the limits. * diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index be08a3a1..9c51cb46 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -412,7 +412,7 @@ cdef class SimplexTree: """This function retrieves good values for extended persistence, and separate the diagrams into the Ordinary, Relative, Extended+ and Extended- subdiagrams. - :param homology_coeff_field: The homology coefficient field. Must be a prime number. Default value is 11. + :param homology_coeff_field: The homology coefficient field. Must be a prime number. Default value is 11. Max is 46337. :type homology_coeff_field: int :param min_persistence: The minimum persistence value (i.e., the absolute value of the difference between the persistence diagram point coordinates) to take into account (strictly greater than min_persistence). @@ -449,7 +449,7 @@ cdef class SimplexTree: """This function computes and returns the persistence of the simplicial complex. :param homology_coeff_field: The homology coefficient field. Must be a - prime number. Default value is 11. + prime number. Default value is 11. Max is 46337. :type homology_coeff_field: int :param min_persistence: The minimum persistence value to take into account (strictly greater than min_persistence). Default value is @@ -472,7 +472,7 @@ cdef class SimplexTree: when you do not want the list :func:`persistence` returns. :param homology_coeff_field: The homology coefficient field. Must be a - prime number. Default value is 11. + prime number. Default value is 11. Max is 46337. :type homology_coeff_field: int :param min_persistence: The minimum persistence value to take into account (strictly greater than min_persistence). Default value is -- cgit v1.2.3 From a93e26976e5898b267d8b743e080e8869ff41b4f Mon Sep 17 00:00:00 2001 From: Hind-M Date: Tue, 27 Jul 2021 11:01:31 +0200 Subject: Remove unnecessary checks for primality Document homology_coeff_field values in cubical --- .../include/gudhi/Persistent_cohomology/Field_Zp.h | 8 ++------ .../test/persistent_cohomology_unit_test.cpp | 5 +++++ src/python/gudhi/cubical_complex.pyx | 4 ++-- src/python/gudhi/periodic_cubical_complex.pyx | 4 ++-- 4 files changed, 11 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h b/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h index 7ecc9a80..8ec89e41 100644 --- a/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h +++ b/src/Persistent_cohomology/include/gudhi/Persistent_cohomology/Field_Zp.h @@ -38,12 +38,12 @@ class Field_Zp { Prime = charac; - // Check that the provided prime is less than the maximum allowed as int and calculation below : 46337 ; i.e (max_prime-1)**2 <= INT_MAX + // Check that the provided prime is less than the maximum allowed as int, calculation below, and 'plus_times_equal' function : 46337 ; i.e (max_prime-1)*max_prime <= INT_MAX if(Prime > 46337) throw std::invalid_argument("Maximum homology_coeff_field allowed value is 46337"); // Check for primality - if ((Prime == 0) || (Prime == 1) || ((Prime > 3) && ((Prime % 2 == 0) || (Prime % 3 == 0)))) + if (Prime <= 1) throw std::invalid_argument("homology_coeff_field must be a prime number"); inverse_.clear(); @@ -59,10 +59,6 @@ class Field_Zp { mult = inv * i; } inverse_.push_back(inv); - if ( (i*i <= Prime) && (((i-5)%6) == 0) ) { - if ((Prime % i == 0) || (Prime % (i + 2) == 0)) - throw std::invalid_argument("homology_coeff_field must be a prime number"); - } } } diff --git a/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp b/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp index 35bb5988..041cb0fd 100644 --- a/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp +++ b/src/Persistent_cohomology/test/persistent_cohomology_unit_test.cpp @@ -176,6 +176,11 @@ BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_5 ) test_rips_persistence_in_dimension(5); } +BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_6 ) +{ + BOOST_CHECK_THROW(test_rips_persistence_in_dimension(6), std::invalid_argument); +} + BOOST_AUTO_TEST_CASE( rips_persistent_cohomology_single_field_dim_11 ) { test_rips_persistence_in_dimension(11); diff --git a/src/python/gudhi/cubical_complex.pyx b/src/python/gudhi/cubical_complex.pyx index adc40499..97c69a2d 100644 --- a/src/python/gudhi/cubical_complex.pyx +++ b/src/python/gudhi/cubical_complex.pyx @@ -147,7 +147,7 @@ cdef class CubicalComplex: :func:`persistence` returns. :param homology_coeff_field: The homology coefficient field. Must be a - prime number + prime number. Default value is 11. Max is 46337. :type homology_coeff_field: int. :param min_persistence: The minimum persistence value to take into account (strictly greater than min_persistence). Default value is @@ -169,7 +169,7 @@ cdef class CubicalComplex: """This function computes and returns the persistence of the complex. :param homology_coeff_field: The homology coefficient field. Must be a - prime number + prime number. Default value is 11. Max is 46337. :type homology_coeff_field: int. :param min_persistence: The minimum persistence value to take into account (strictly greater than min_persistence). Default value is diff --git a/src/python/gudhi/periodic_cubical_complex.pyx b/src/python/gudhi/periodic_cubical_complex.pyx index 0eaa5867..ef1d3080 100644 --- a/src/python/gudhi/periodic_cubical_complex.pyx +++ b/src/python/gudhi/periodic_cubical_complex.pyx @@ -148,7 +148,7 @@ cdef class PeriodicCubicalComplex: :func:`persistence` returns. :param homology_coeff_field: The homology coefficient field. Must be a - prime number + prime number. Default value is 11. Max is 46337. :type homology_coeff_field: int. :param min_persistence: The minimum persistence value to take into account (strictly greater than min_persistence). Default value is @@ -170,7 +170,7 @@ cdef class PeriodicCubicalComplex: """This function computes and returns the persistence of the complex. :param homology_coeff_field: The homology coefficient field. Must be a - prime number + prime number. Default value is 11. Max is 46337. :type homology_coeff_field: int. :param min_persistence: The minimum persistence value to take into account (strictly greater than min_persistence). Default value is -- cgit v1.2.3