From fda0084941ece5d41a258d19ca4eb0b3d87384a4 Mon Sep 17 00:00:00 2001 From: ROUVREAU Vincent Date: Wed, 9 Dec 2020 09:41:13 +0100 Subject: Fix #388 --- src/python/gudhi/simplex_tree.pxd | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/python/gudhi/simplex_tree.pxd') diff --git a/src/python/gudhi/simplex_tree.pxd b/src/python/gudhi/simplex_tree.pxd index 3c4cbed3..283830ff 100644 --- a/src/python/gudhi/simplex_tree.pxd +++ b/src/python/gudhi/simplex_tree.pxd @@ -74,6 +74,8 @@ cdef extern from "Simplex_tree_interface.h" namespace "Gudhi": Simplex_tree_skeleton_iterator get_skeleton_iterator_begin(int dimension) nogil Simplex_tree_skeleton_iterator get_skeleton_iterator_end(int dimension) nogil pair[Simplex_tree_boundary_iterator, Simplex_tree_boundary_iterator] get_boundary_iterators(vector[int] simplex) nogil except + + + cdef int _GUDHI_USE_EIGEN3 cdef extern from "Persistent_cohomology_interface.h" namespace "Gudhi": cdef cppclass Simplex_tree_persistence_interface "Gudhi::Persistent_cohomology_interface>": -- cgit v1.2.3 From 957da77f9484972ce34d0415502887f92080878e Mon Sep 17 00:00:00 2001 From: ROUVREAU Vincent Date: Fri, 11 Dec 2020 09:38:12 +0100 Subject: code review: GUDHI_USE_EIGEN3 generated by CMake in __init__.py as suggested and roll back the other solution --- src/python/CMakeLists.txt | 2 ++ src/python/gudhi/__init__.py.in | 4 ++++ src/python/gudhi/simplex_tree.pxd | 2 -- src/python/gudhi/simplex_tree.pyx | 3 --- src/python/include/Simplex_tree_interface.h | 6 ------ src/python/test/test_simplex_tree.py | 4 ++-- 6 files changed, 8 insertions(+), 13 deletions(-) (limited to 'src/python/gudhi/simplex_tree.pxd') diff --git a/src/python/CMakeLists.txt b/src/python/CMakeLists.txt index 45c89609..5a245aac 100644 --- a/src/python/CMakeLists.txt +++ b/src/python/CMakeLists.txt @@ -129,11 +129,13 @@ if(PYTHONINTERP_FOUND) set(GUDHI_PYTHON_EXTRA_COMPILE_ARGS "${GUDHI_PYTHON_EXTRA_COMPILE_ARGS}'-DDEBUG_TRACES', ") endif() + set(GUDHI_USE_EIGEN3 "False") if (EIGEN3_FOUND) add_gudhi_debug_info("Eigen3 version ${EIGEN3_VERSION}") # No problem, even if no CGAL found set(GUDHI_PYTHON_EXTRA_COMPILE_ARGS "${GUDHI_PYTHON_EXTRA_COMPILE_ARGS}'-DCGAL_EIGEN3_ENABLED', ") set(GUDHI_PYTHON_EXTRA_COMPILE_ARGS "${GUDHI_PYTHON_EXTRA_COMPILE_ARGS}'-DGUDHI_USE_EIGEN3', ") + set(GUDHI_USE_EIGEN3 "True") endif (EIGEN3_FOUND) set(GUDHI_CYTHON_MODULES "${GUDHI_CYTHON_MODULES}'off_reader', ") diff --git a/src/python/gudhi/__init__.py.in b/src/python/gudhi/__init__.py.in index 79e12fbc..3043201a 100644 --- a/src/python/gudhi/__init__.py.in +++ b/src/python/gudhi/__init__.py.in @@ -23,6 +23,10 @@ __all__ = [@GUDHI_PYTHON_MODULES@ @GUDHI_PYTHON_MODULES_EXTRA@] __available_modules = '' __missing_modules = '' +# For unitary tests purpose +# could use "if 'collapse_edges' in gudhi.__all__" when collapse edges will have a python module +__GUDHI_USE_EIGEN3 = @GUDHI_USE_EIGEN3@ + # Try to import * from gudhi.__module_name for default modules. # Extra modules require an explicit import by the user (mostly because of # unusual dependencies, but also to avoid cluttering namespace gudhi and diff --git a/src/python/gudhi/simplex_tree.pxd b/src/python/gudhi/simplex_tree.pxd index 283830ff..3c4cbed3 100644 --- a/src/python/gudhi/simplex_tree.pxd +++ b/src/python/gudhi/simplex_tree.pxd @@ -74,8 +74,6 @@ cdef extern from "Simplex_tree_interface.h" namespace "Gudhi": Simplex_tree_skeleton_iterator get_skeleton_iterator_begin(int dimension) nogil Simplex_tree_skeleton_iterator get_skeleton_iterator_end(int dimension) nogil pair[Simplex_tree_boundary_iterator, Simplex_tree_boundary_iterator] get_boundary_iterators(vector[int] simplex) nogil except + - - cdef int _GUDHI_USE_EIGEN3 cdef extern from "Persistent_cohomology_interface.h" namespace "Gudhi": cdef cppclass Simplex_tree_persistence_interface "Gudhi::Persistent_cohomology_interface>": diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index eb44c0fc..7d6ab89a 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -17,9 +17,6 @@ __author__ = "Vincent Rouvreau" __copyright__ = "Copyright (C) 2016 Inria" __license__ = "MIT" -# For unitary tests purpose -__GUDHI_USE_EIGEN3 = _GUDHI_USE_EIGEN3 - # SimplexTree python interface cdef class SimplexTree: """The simplex tree is an efficient and flexible data structure for diff --git a/src/python/include/Simplex_tree_interface.h b/src/python/include/Simplex_tree_interface.h index 50592e25..82444609 100644 --- a/src/python/include/Simplex_tree_interface.h +++ b/src/python/include/Simplex_tree_interface.h @@ -27,12 +27,6 @@ namespace Gudhi { -#ifdef GUDHI_USE_EIGEN3 -const int _GUDHI_USE_EIGEN3 = 1; -#else -const int _GUDHI_USE_EIGEN3 = 0; -#endif - template class Simplex_tree_interface : public Simplex_tree { public: diff --git a/src/python/test/test_simplex_tree.py b/src/python/test/test_simplex_tree.py index 15b472ee..0c072baa 100755 --- a/src/python/test/test_simplex_tree.py +++ b/src/python/test/test_simplex_tree.py @@ -8,7 +8,7 @@ - YYYY/MM Author: Description of the modification """ -from gudhi import SimplexTree, simplex_tree +from gudhi import SimplexTree, __GUDHI_USE_EIGEN3 import pytest __author__ = "Vincent Rouvreau" @@ -355,7 +355,7 @@ def test_collapse_edges(): # If no Eigen3, collapse_edges just return a copy, no action. Maybe it would require some user warning st.collapse_edges() - if simplex_tree.__GUDHI_USE_EIGEN3: + if __GUDHI_USE_EIGEN3: assert st.num_simplices() == 9 assert st.find([1, 3]) == False for simplex in st.get_skeleton(0): -- cgit v1.2.3 From 40e0976e8dae27f0e30f3c9df1fd7de1a7343948 Mon Sep 17 00:00:00 2001 From: ROUVREAU Vincent Date: Fri, 11 Dec 2020 10:31:09 +0100 Subject: code review: throw an eception if collapse_edges when no Eigen3 --- src/python/gudhi/simplex_tree.pxd | 2 +- src/python/gudhi/simplex_tree.pyx | 4 ++-- src/python/include/Simplex_tree_interface.h | 4 +--- src/python/test/test_simplex_tree.py | 7 ++++--- 4 files changed, 8 insertions(+), 9 deletions(-) (limited to 'src/python/gudhi/simplex_tree.pxd') diff --git a/src/python/gudhi/simplex_tree.pxd b/src/python/gudhi/simplex_tree.pxd index 3c4cbed3..000323af 100644 --- a/src/python/gudhi/simplex_tree.pxd +++ b/src/python/gudhi/simplex_tree.pxd @@ -63,7 +63,7 @@ cdef extern from "Simplex_tree_interface.h" namespace "Gudhi": bool make_filtration_non_decreasing() nogil void compute_extended_filtration() nogil vector[vector[pair[int, pair[double, double]]]] compute_extended_persistence_subdiagrams(vector[pair[int, pair[double, double]]] dgm, double min_persistence) nogil - Simplex_tree_interface_full_featured* collapse_edges(int nb_collapse_iteration) nogil + Simplex_tree_interface_full_featured* collapse_edges(int nb_collapse_iteration) nogil except + void reset_filtration(double filtration, int dimension) nogil # Iterators over Simplex tree pair[vector[int], double] get_simplex_and_filtration(Simplex_tree_simplex_handle f_simplex) nogil diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index 7d6ab89a..665d41e6 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -628,8 +628,8 @@ cdef class SimplexTree: :param nb_iterations: The number of edge collapse iterations to perform. Default is 1. :type nb_iterations: int - :note: collapse_edges function requires `Eigen `_ >= 3.1.0, otherwise no action is - performed. + :note: collapse_edges method requires `Eigen `_ >= 3.1.0 and an exception is thrown + if this method is not available. """ # Backup old pointer cdef Simplex_tree_interface_full_featured* ptr = self.get_ptr() diff --git a/src/python/include/Simplex_tree_interface.h b/src/python/include/Simplex_tree_interface.h index 82444609..629f6083 100644 --- a/src/python/include/Simplex_tree_interface.h +++ b/src/python/include/Simplex_tree_interface.h @@ -191,9 +191,7 @@ class Simplex_tree_interface : public Simplex_tree { } return collapsed_stree_ptr; #else - // If no Eigen3, return a copy, as it will be deleted in pyx - Simplex_tree_interface* collapsed_stree_ptr = new Simplex_tree_interface(*this); - return collapsed_stree_ptr; + throw std::runtime_error("Unable to collapse edges as it requires Eigen3 >= 3.1.0."); #endif } diff --git a/src/python/test/test_simplex_tree.py b/src/python/test/test_simplex_tree.py index 0c072baa..a3eacaa9 100755 --- a/src/python/test/test_simplex_tree.py +++ b/src/python/test/test_simplex_tree.py @@ -353,15 +353,16 @@ def test_collapse_edges(): assert st.num_simplices() == 10 - # If no Eigen3, collapse_edges just return a copy, no action. Maybe it would require some user warning - st.collapse_edges() if __GUDHI_USE_EIGEN3: + st.collapse_edges() assert st.num_simplices() == 9 assert st.find([1, 3]) == False for simplex in st.get_skeleton(0): assert simplex[1] == 1. else: - assert st.num_simplices() == 10 + # If no Eigen3, collapse_edges throws an exception + with pytest.raises(RuntimeError): + st.collapse_edges() def test_reset_filtration(): st = SimplexTree() -- cgit v1.2.3