From cea821f9ca34c270a5ccc047342c2c21ae79a6c0 Mon Sep 17 00:00:00 2001 From: ROUVREAU Vincent Date: Fri, 28 Aug 2020 17:42:12 +0200 Subject: A prototype to fix #364 --- src/python/gudhi/simplex_tree.pyx | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index dfb1d985..e297cfbd 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -17,6 +17,9 @@ __author__ = "Vincent Rouvreau" __copyright__ = "Copyright (C) 2016 Inria" __license__ = "MIT" +cdef bool callback(vector[int] simplex, void *blocker_func): + return (blocker_func)(simplex) + # SimplexTree python interface cdef class SimplexTree: """The simplex tree is an efficient and flexible data structure for @@ -409,6 +412,24 @@ cdef class SimplexTree: persistence_result = self.pcohptr.get_persistence() return self.get_ptr().compute_extended_persistence_subdiagrams(persistence_result, min_persistence) + def expansion_with_blocker(self, max_dim, blocker_func): + """Expands the Simplex_tree containing only its one skeleton + until dimension max_dim. + + The expanded simplicial complex until dimension :math:`d` + attached to a graph :math:`G` is the maximal simplicial complex of + dimension at most :math:`d` admitting the graph :math:`G` as + :math:`1`-skeleton. + The filtration value assigned to a simplex is the maximal filtration + value of one of its edges. + + The Simplex_tree must contain no simplex of dimension bigger than + 1 when calling the method. + + :param max_dim: The maximal dimension. + :type max_dim: int + """ + self.get_ptr().expansion_with_blockers_callback(max_dim, callback, blocker_func) def persistence(self, homology_coeff_field=11, min_persistence=0, persistence_dim_max = False): """This function computes and returns the persistence of the simplicial complex. -- cgit v1.2.3 From c2eb0484191f89fcbe40bc4ab04943eb808f12a9 Mon Sep 17 00:00:00 2001 From: ROUVREAU Vincent Date: Tue, 1 Sep 2020 14:51:25 +0200 Subject: expansion with blocker and how to modify filtration value --- src/python/gudhi/simplex_tree.pxd | 2 +- src/python/gudhi/simplex_tree.pyx | 23 +++++++++--------- src/python/test/test_simplex_tree.py | 46 +++++++++++++++++++++--------------- 3 files changed, 40 insertions(+), 31 deletions(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pxd b/src/python/gudhi/simplex_tree.pxd index 44533d7f..80c6ffca 100644 --- a/src/python/gudhi/simplex_tree.pxd +++ b/src/python/gudhi/simplex_tree.pxd @@ -66,7 +66,7 @@ cdef extern from "Simplex_tree_interface.h" namespace "Gudhi": vector[Simplex_tree_simplex_handle].const_iterator get_filtration_iterator_end() nogil Simplex_tree_skeleton_iterator get_skeleton_iterator_begin(int dimension) nogil Simplex_tree_skeleton_iterator get_skeleton_iterator_end(int dimension) nogil - # + # Expansion with blockers ctypedef bool (*blocker_func)(vector[int], void *user_data) void expansion_with_blockers_callback(int dimension, blocker_func user_func, void *user_data) diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index e297cfbd..debe92c0 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -413,21 +413,22 @@ cdef class SimplexTree: return self.get_ptr().compute_extended_persistence_subdiagrams(persistence_result, min_persistence) def expansion_with_blocker(self, max_dim, blocker_func): - """Expands the Simplex_tree containing only its one skeleton - until dimension max_dim. + """Expands the Simplex_tree containing only a graph. Simplices corresponding to cliques in the graph are added + incrementally, faces before cofaces, unless the simplex has dimension larger than `max_dim` or `blocker_func` + returns `True` for this simplex. - The expanded simplicial complex until dimension :math:`d` - attached to a graph :math:`G` is the maximal simplicial complex of - dimension at most :math:`d` admitting the graph :math:`G` as - :math:`1`-skeleton. - The filtration value assigned to a simplex is the maximal filtration - value of one of its edges. + The function identifies a candidate simplex whose faces are all already in the complex, inserts it with a + filtration value corresponding to the maximum of the filtration values of the faces, then calls `blocker_func` + with this new simplex (represented as a list of int). If `blocker_func` returns `True`, the simplex is removed, + otherwise it is kept. The algorithm then proceeds with the next candidate. - The Simplex_tree must contain no simplex of dimension bigger than - 1 when calling the method. + Note that you cannot update the filtration value of the simplex during the evaluation of `blocker_func`, as it + would segfault. - :param max_dim: The maximal dimension. + :param max_dim: Expansion maximal dimension value. :type max_dim: int + :param blocker_func: Blocker oracle. + :type blocker_func: Its concept is `Boolean blocker_func(list of int)` """ self.get_ptr().expansion_with_blockers_callback(max_dim, callback, blocker_func) diff --git a/src/python/test/test_simplex_tree.py b/src/python/test/test_simplex_tree.py index 7aad8259..33b0ac99 100755 --- a/src/python/test/test_simplex_tree.py +++ b/src/python/test/test_simplex_tree.py @@ -359,16 +359,6 @@ def test_collapse_edges(): for simplex in st.get_skeleton(0): assert simplex[1] == 1. -def blocker(simplex): - try: - # Block all simplices that countains vertex 6 - simplex.index(6) - print(simplex, ' is blocked') - return True - except ValueError: - print(simplex, ' is accepted') - return False - def test_expansion_with_blocker(): st=SimplexTree() st.insert([0,1],0) @@ -384,21 +374,39 @@ def test_expansion_with_blocker(): st.insert([5,6],10) st.insert([6],10) + # One cannot modify filtration inside blocker - segfault - Let's record accepted simplices + accepted_simp = [] + def blocker(simplex): + try: + # Block all simplices that countains vertex 6 + simplex.index(6) + print(simplex, ' is blocked') + return True + except ValueError: + print(simplex, ' is accepted') + accepted_simp.append(simplex) + return False + st.expansion_with_blocker(2, blocker) + for simplex in accepted_simp: + st.assign_filtration(simplex, st.filtration(simplex) + 1.) assert st.num_simplices() == 22 assert st.dimension() == 2 assert st.find([4,5,6]) == False - assert st.filtration([0,1,2]) == 3. - assert st.filtration([0,1,3]) == 4. - assert st.filtration([0,2,3]) == 5. - assert st.filtration([1,2,3]) == 5. + assert st.filtration([0,1,2]) == 4. + assert st.filtration([0,1,3]) == 5. + assert st.filtration([0,2,3]) == 6. + assert st.filtration([1,2,3]) == 6. + accepted_simp = [] st.expansion_with_blocker(3, blocker) + for simplex in accepted_simp: + st.assign_filtration(simplex, st.filtration(simplex) + 1.) assert st.num_simplices() == 23 assert st.dimension() == 3 assert st.find([4,5,6]) == False - assert st.filtration([0,1,2]) == 3. - assert st.filtration([0,1,3]) == 4. - assert st.filtration([0,2,3]) == 5. - assert st.filtration([1,2,3]) == 5. - assert st.filtration([0,1,2,3]) == 5. + assert st.filtration([0,1,2]) == 4. + assert st.filtration([0,1,3]) == 5. + assert st.filtration([0,2,3]) == 6. + assert st.filtration([1,2,3]) == 6. + assert st.filtration([0,1,2,3]) == 6. -- cgit v1.2.3 From f4f5992e9025d055e35c86589923f1b0299f552e Mon Sep 17 00:00:00 2001 From: ROUVREAU Vincent Date: Thu, 22 Apr 2021 08:39:03 +0200 Subject: code review: rename block_func when it is a type. Type is a 'callable' --- src/python/gudhi/simplex_tree.pxd | 4 ++-- src/python/gudhi/simplex_tree.pyx | 2 +- src/python/include/Simplex_tree_interface.h | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pxd b/src/python/gudhi/simplex_tree.pxd index 80c6ffca..ff750af9 100644 --- a/src/python/gudhi/simplex_tree.pxd +++ b/src/python/gudhi/simplex_tree.pxd @@ -67,8 +67,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 # Expansion with blockers - ctypedef bool (*blocker_func)(vector[int], void *user_data) - void expansion_with_blockers_callback(int dimension, blocker_func user_func, void *user_data) + ctypedef bool (*blocker_func_t)(vector[int], void *user_data) + void expansion_with_blockers_callback(int dimension, blocker_func_t user_func, void *user_data) 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 debe92c0..383d1949 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -428,7 +428,7 @@ cdef class SimplexTree: :param max_dim: Expansion maximal dimension value. :type max_dim: int :param blocker_func: Blocker oracle. - :type blocker_func: Its concept is `Boolean blocker_func(list of int)` + :type blocker_func: Callable[[List[int]], bool] """ self.get_ptr().expansion_with_blockers_callback(max_dim, callback, blocker_func) diff --git a/src/python/include/Simplex_tree_interface.h b/src/python/include/Simplex_tree_interface.h index 9f92b349..a859bec0 100644 --- a/src/python/include/Simplex_tree_interface.h +++ b/src/python/include/Simplex_tree_interface.h @@ -39,7 +39,7 @@ class Simplex_tree_interface : public Simplex_tree { using Skeleton_simplex_iterator = typename Base::Skeleton_simplex_iterator; using Complex_simplex_iterator = typename Base::Complex_simplex_iterator; using Extended_filtration_data = typename Base::Extended_filtration_data; - typedef bool (*blocker_func)(Simplex simplex, void *user_data); + typedef bool (*blocker_func_t)(Simplex simplex, void *user_data); public: @@ -189,7 +189,7 @@ class Simplex_tree_interface : public Simplex_tree { return collapsed_stree_ptr; } - void expansion_with_blockers_callback(int dimension, blocker_func user_func, void *user_data) { + void expansion_with_blockers_callback(int dimension, blocker_func_t user_func, void *user_data) { Base::expansion_with_blockers(dimension, [&](Simplex_handle sh){ Simplex simplex; for (auto vertex : Base::simplex_vertex_range(sh)) { -- cgit v1.2.3 From 37a141533397568e7070c734e21ef9c4dc85d132 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Thu, 10 Feb 2022 16:16:14 +0100 Subject: Add SimplexTree copy method and its test --- src/python/gudhi/simplex_tree.pxd | 1 + src/python/gudhi/simplex_tree.pyx | 14 ++++++++++++++ src/python/test/test_simplex_tree.py | 19 ++++++++++++++++++- 3 files changed, 33 insertions(+), 1 deletion(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pxd b/src/python/gudhi/simplex_tree.pxd index 006a24ed..92139db4 100644 --- a/src/python/gudhi/simplex_tree.pxd +++ b/src/python/gudhi/simplex_tree.pxd @@ -45,6 +45,7 @@ cdef extern from "Simplex_tree_interface.h" namespace "Gudhi": cdef cppclass Simplex_tree_interface_full_featured "Gudhi::Simplex_tree_interface": Simplex_tree_interface_full_featured() nogil + Simplex_tree_interface_full_featured(Simplex_tree_interface_full_featured&) nogil double simplex_filtration(vector[int] simplex) nogil void assign_simplex_filtration(vector[int] simplex, double filtration) nogil void initialize_filtration() nogil diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index c3720936..6b3116a4 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -63,6 +63,20 @@ cdef class SimplexTree: """ return self.pcohptr != NULL + def copy(self): + """ + :returns: A simplex tree that is a deep copy itself. + :rtype: SimplexTree + """ + stree = SimplexTree() + cdef Simplex_tree_interface_full_featured* stree_ptr + cdef Simplex_tree_interface_full_featured* self_ptr=self.get_ptr() + with nogil: + stree_ptr = new Simplex_tree_interface_full_featured(dereference(self_ptr)) + + stree.thisptr = (stree_ptr) + return stree + def filtration(self, simplex): """This function returns the filtration value for a given N-simplex in this simplicial complex, or +infinity if it is not in the complex. diff --git a/src/python/test/test_simplex_tree.py b/src/python/test/test_simplex_tree.py index 31c46213..dac45288 100755 --- a/src/python/test/test_simplex_tree.py +++ b/src/python/test/test_simplex_tree.py @@ -447,4 +447,21 @@ def test_persistence_intervals_in_dimension(): assert np.array_equal(H2, np.array([[ 0., float("inf")]])) # Test empty case assert st.persistence_intervals_in_dimension(3).shape == (0, 2) - \ No newline at end of file + +def test_simplex_tree_copy(): + st = SimplexTree() + st .insert([1,2,3], 0.) + a = st.copy() + # TODO(VR): when #463 is merged, replace with + # assert a == st + assert a.num_vertices() == st.num_vertices() + assert a.num_simplices() == st.num_simplices() + st_filt_list = list(st.get_filtration()) + assert list(a.get_filtration()) == st_filt_list + + a.remove_maximal_simplex([1, 2, 3]) + a_filt_list = list(a.get_filtration()) + assert len(a_filt_list) < len(st_filt_list) + + for a_splx in a_filt_list: + assert a_splx in st_filt_list -- cgit v1.2.3 From fb8ce008feadcaf6a936740a3ed54d50970c731c Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Fri, 11 Feb 2022 23:11:26 +0100 Subject: __copy__, __deepcopy__, copy, and copy ctors. Still pb with the doc --- .github/next_release.md | 3 ++ src/python/gudhi/simplex_tree.pyx | 54 +++++++++++++++---- src/python/test/test_simplex_tree.py | 102 +++++++++++++++++++++++++++++++---- 3 files changed, 140 insertions(+), 19 deletions(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/.github/next_release.md b/.github/next_release.md index e21b25c7..3946404b 100644 --- a/.github/next_release.md +++ b/.github/next_release.md @@ -13,6 +13,9 @@ Below is a list of changes made since GUDHI 3.5.0: - [Representations](https://gudhi.inria.fr/python/latest/representations.html#gudhi.representations.vector_methods.BettiCurve) - A more flexible Betti curve class capable of computing exact curves +- [Simplex tree](https://gudhi.inria.fr/python/latest/simplex_tree_ref.html) + - `__copy__`, `__deepcopy__`, `copy` and copy constructors + - Installation - Boost ≥ 1.66.0 is now required (was ≥ 1.56.0). diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index 6b3116a4..ed7c3b92 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -30,6 +30,7 @@ cdef class SimplexTree: # unfortunately 'cdef public Simplex_tree_interface_full_featured* thisptr' is not possible # Use intptr_t instead to cast the pointer cdef public intptr_t thisptr + cdef bool __thisptr_to_be_deleted # Get the pointer casted as it should be cdef Simplex_tree_interface_full_featured* get_ptr(self) nogil: @@ -38,17 +39,36 @@ cdef class SimplexTree: cdef Simplex_tree_persistence_interface * pcohptr # Fake constructor that does nothing but documenting the constructor - def __init__(self): + def __init__(self, other = None, copy = True): """SimplexTree constructor. + :param other: If `other` is a SimplexTree (default = None), the SimplexTree is constructed from a deep/shallow copy of `other`. + :type other: SimplexTree + :param copy: If `True`, the copy will be deep and if `False, the copy will be shallow. Default is `True`. + :type copy: bool + :returns: A simplex tree that is a (deep or shallow) copy of itself. + :rtype: SimplexTree + :note: copy constructor requires :func:`compute_persistence` to be launched again as the result is not copied. """ # The real cython constructor - def __cinit__(self): - self.thisptr = (new Simplex_tree_interface_full_featured()) + def __cinit__(self, other = None, copy = True): + cdef SimplexTree ostr + if other and type(other) is SimplexTree: + ostr = other + if copy: + self.thisptr = (new Simplex_tree_interface_full_featured(dereference(ostr.get_ptr()))) + else: + self.thisptr = ostr.thisptr + # Avoid double free - The original is in charge of deletion + self.__thisptr_to_be_deleted = False + else: + self.__thisptr_to_be_deleted = True + self.thisptr = (new Simplex_tree_interface_full_featured()) def __dealloc__(self): cdef Simplex_tree_interface_full_featured* ptr = self.get_ptr() - if ptr != NULL: + # Avoid double free - The original is in charge of deletion + if ptr != NULL and self.__thisptr_to_be_deleted: del ptr if self.pcohptr != NULL: del self.pcohptr @@ -63,20 +83,34 @@ cdef class SimplexTree: """ return self.pcohptr != NULL - def copy(self): + def copy(self, deep=True): """ - :returns: A simplex tree that is a deep copy itself. + :param deep: If `True`, the copy will be deep and if `False`, the copy will be shallow. Default is `True`. + :type deep: bool + :returns: A simplex tree that is a (deep or shallow) copy of itself. :rtype: SimplexTree + :note: copy requires :func:`compute_persistence` to be launched again as the result is not copied. """ stree = SimplexTree() cdef Simplex_tree_interface_full_featured* stree_ptr cdef Simplex_tree_interface_full_featured* self_ptr=self.get_ptr() - with nogil: - stree_ptr = new Simplex_tree_interface_full_featured(dereference(self_ptr)) - - stree.thisptr = (stree_ptr) + if deep: + with nogil: + stree_ptr = new Simplex_tree_interface_full_featured(dereference(self_ptr)) + + stree.thisptr = (stree_ptr) + else: + stree.thisptr = self.thisptr + # Avoid double free - The original is in charge of deletion + stree.__thisptr_to_be_deleted = False return stree + def __copy__(self): + return self.copy(deep=False) + + def __deepcopy__(self): + return self.copy(deep=True) + def filtration(self, simplex): """This function returns the filtration value for a given N-simplex in this simplicial complex, or +infinity if it is not in the complex. diff --git a/src/python/test/test_simplex_tree.py b/src/python/test/test_simplex_tree.py index dac45288..6db6d8fb 100755 --- a/src/python/test/test_simplex_tree.py +++ b/src/python/test/test_simplex_tree.py @@ -448,20 +448,104 @@ def test_persistence_intervals_in_dimension(): # Test empty case assert st.persistence_intervals_in_dimension(3).shape == (0, 2) -def test_simplex_tree_copy(): +def test_simplex_tree_deep_copy(): st = SimplexTree() - st .insert([1,2,3], 0.) - a = st.copy() + st.insert([1, 2, 3], 0.) + # persistence is not copied + st.compute_persistence() + + st_copy = st.copy(deep=True) # TODO(VR): when #463 is merged, replace with - # assert a == st - assert a.num_vertices() == st.num_vertices() - assert a.num_simplices() == st.num_simplices() + # assert st_copy == st + assert st_copy.num_vertices() == st.num_vertices() + assert st_copy.num_simplices() == st.num_simplices() st_filt_list = list(st.get_filtration()) - assert list(a.get_filtration()) == st_filt_list + assert list(st_copy.get_filtration()) == st_filt_list + + assert st.__is_persistence_defined() == True + assert st_copy.__is_persistence_defined() == False - a.remove_maximal_simplex([1, 2, 3]) - a_filt_list = list(a.get_filtration()) + st_copy.remove_maximal_simplex([1, 2, 3]) + a_filt_list = list(st_copy.get_filtration()) assert len(a_filt_list) < len(st_filt_list) for a_splx in a_filt_list: assert a_splx in st_filt_list + + # test double free + del st + del st_copy + +def test_simplex_tree_shallow_copy(): + st = SimplexTree() + st.insert([1, 2, 3], 0.) + # persistence is not copied + st.compute_persistence() + + st_copy = st.copy(deep=False) + # TODO(VR): when #463 is merged, replace with + # assert st_copy == st + assert st_copy.num_vertices() == st.num_vertices() + assert st_copy.num_simplices() == st.num_simplices() + assert list(st_copy.get_filtration()) == list(st.get_filtration()) + + assert st.__is_persistence_defined() == True + assert st_copy.__is_persistence_defined() == False + + st_copy.assign_filtration([1, 2, 3], 2.) + assert list(st_copy.get_filtration()) == list(st.get_filtration()) + + # test double free + del st + del st_copy + +def test_simplex_tree_deep_copy_constructor(): + st = SimplexTree() + st.insert([1, 2, 3], 0.) + # persistence is not copied + st.compute_persistence() + + st_copy = SimplexTree(st, copy = True) + # TODO(VR): when #463 is merged, replace with + # assert st_copy == st + assert st_copy.num_vertices() == st.num_vertices() + assert st_copy.num_simplices() == st.num_simplices() + st_filt_list = list(st.get_filtration()) + assert list(st_copy.get_filtration()) == st_filt_list + + assert st.__is_persistence_defined() == True + assert st_copy.__is_persistence_defined() == False + + st_copy.remove_maximal_simplex([1, 2, 3]) + a_filt_list = list(st_copy.get_filtration()) + assert len(a_filt_list) < len(st_filt_list) + + for a_splx in a_filt_list: + assert a_splx in st_filt_list + + # test double free + del st + del st_copy + +def test_simplex_tree_shallow_copy(): + st = SimplexTree() + st.insert([1, 2, 3], 0.) + # persistence is not copied + st.compute_persistence() + + st_copy = SimplexTree(st, copy = False) + # TODO(VR): when #463 is merged, replace with + # assert st_copy == st + assert st_copy.num_vertices() == st.num_vertices() + assert st_copy.num_simplices() == st.num_simplices() + assert list(st_copy.get_filtration()) == list(st.get_filtration()) + + assert st.__is_persistence_defined() == True + assert st_copy.__is_persistence_defined() == False + + st_copy.assign_filtration([1, 2, 3], 2.) + assert list(st_copy.get_filtration()) == list(st.get_filtration()) + + # test double free + del st + del st_copy -- cgit v1.2.3 From 43981a4d487669fe2002337ab62b72dd9e83a64a Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Mon, 14 Feb 2022 11:08:00 +0100 Subject: Remove shallow copy --- .github/next_release.md | 2 +- src/python/gudhi/simplex_tree.pyx | 54 +++++++++++++----------------------- src/python/test/test_simplex_tree.py | 50 ++------------------------------- 3 files changed, 23 insertions(+), 83 deletions(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/.github/next_release.md b/.github/next_release.md index 3946404b..3d4761eb 100644 --- a/.github/next_release.md +++ b/.github/next_release.md @@ -14,7 +14,7 @@ Below is a list of changes made since GUDHI 3.5.0: - A more flexible Betti curve class capable of computing exact curves - [Simplex tree](https://gudhi.inria.fr/python/latest/simplex_tree_ref.html) - - `__copy__`, `__deepcopy__`, `copy` and copy constructors + - `__deepcopy__`, `copy` and copy constructors - Installation - Boost ≥ 1.66.0 is now required (was ≥ 1.56.0). diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index ed7c3b92..0213e363 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -30,7 +30,6 @@ cdef class SimplexTree: # unfortunately 'cdef public Simplex_tree_interface_full_featured* thisptr' is not possible # Use intptr_t instead to cast the pointer cdef public intptr_t thisptr - cdef bool __thisptr_to_be_deleted # Get the pointer casted as it should be cdef Simplex_tree_interface_full_featured* get_ptr(self) nogil: @@ -39,36 +38,32 @@ cdef class SimplexTree: cdef Simplex_tree_persistence_interface * pcohptr # Fake constructor that does nothing but documenting the constructor - def __init__(self, other = None, copy = True): + def __init__(self, other = None): """SimplexTree constructor. - :param other: If `other` is a SimplexTree (default = None), the SimplexTree is constructed from a deep/shallow copy of `other`. + + :param other: If `other` is a `None` (default value), an empty `SimplexTree` is created. + If `other` is a `SimplexTree`, the `SimplexTree` is constructed from a deep copy of `other`. :type other: SimplexTree - :param copy: If `True`, the copy will be deep and if `False, the copy will be shallow. Default is `True`. - :type copy: bool - :returns: A simplex tree that is a (deep or shallow) copy of itself. + :returns: An empty or a copy simplex tree. :rtype: SimplexTree - :note: copy constructor requires :func:`compute_persistence` to be launched again as the result is not copied. + + :note: If the `SimplexTree` is a copy, it requires :func:`compute_persistence` to be launched again as the + persistence result is not copied. """ # The real cython constructor - def __cinit__(self, other = None, copy = True): + def __cinit__(self, other = None): cdef SimplexTree ostr if other and type(other) is SimplexTree: ostr = other - if copy: - self.thisptr = (new Simplex_tree_interface_full_featured(dereference(ostr.get_ptr()))) - else: - self.thisptr = ostr.thisptr - # Avoid double free - The original is in charge of deletion - self.__thisptr_to_be_deleted = False + self.thisptr = (new Simplex_tree_interface_full_featured(dereference(ostr.get_ptr()))) else: - self.__thisptr_to_be_deleted = True self.thisptr = (new Simplex_tree_interface_full_featured()) def __dealloc__(self): cdef Simplex_tree_interface_full_featured* ptr = self.get_ptr() # Avoid double free - The original is in charge of deletion - if ptr != NULL and self.__thisptr_to_be_deleted: + if ptr != NULL: del ptr if self.pcohptr != NULL: del self.pcohptr @@ -83,33 +78,24 @@ cdef class SimplexTree: """ return self.pcohptr != NULL - def copy(self, deep=True): + def copy(self): """ - :param deep: If `True`, the copy will be deep and if `False`, the copy will be shallow. Default is `True`. - :type deep: bool - :returns: A simplex tree that is a (deep or shallow) copy of itself. + :returns: A simplex tree that is a deep copy of itself. :rtype: SimplexTree - :note: copy requires :func:`compute_persistence` to be launched again as the result is not copied. + + :note: copy requires :func:`compute_persistence` to be launched again as the persistence result is not copied. """ stree = SimplexTree() cdef Simplex_tree_interface_full_featured* stree_ptr cdef Simplex_tree_interface_full_featured* self_ptr=self.get_ptr() - if deep: - with nogil: - stree_ptr = new Simplex_tree_interface_full_featured(dereference(self_ptr)) - - stree.thisptr = (stree_ptr) - else: - stree.thisptr = self.thisptr - # Avoid double free - The original is in charge of deletion - stree.__thisptr_to_be_deleted = False - return stree + with nogil: + stree_ptr = new Simplex_tree_interface_full_featured(dereference(self_ptr)) - def __copy__(self): - return self.copy(deep=False) + stree.thisptr = (stree_ptr) + return stree def __deepcopy__(self): - return self.copy(deep=True) + return self.copy() def filtration(self, simplex): """This function returns the filtration value for a given N-simplex in diff --git a/src/python/test/test_simplex_tree.py b/src/python/test/test_simplex_tree.py index 6db6d8fb..62dcc865 100755 --- a/src/python/test/test_simplex_tree.py +++ b/src/python/test/test_simplex_tree.py @@ -454,7 +454,7 @@ def test_simplex_tree_deep_copy(): # persistence is not copied st.compute_persistence() - st_copy = st.copy(deep=True) + st_copy = st.copy() # TODO(VR): when #463 is merged, replace with # assert st_copy == st assert st_copy.num_vertices() == st.num_vertices() @@ -476,36 +476,13 @@ def test_simplex_tree_deep_copy(): del st del st_copy -def test_simplex_tree_shallow_copy(): - st = SimplexTree() - st.insert([1, 2, 3], 0.) - # persistence is not copied - st.compute_persistence() - - st_copy = st.copy(deep=False) - # TODO(VR): when #463 is merged, replace with - # assert st_copy == st - assert st_copy.num_vertices() == st.num_vertices() - assert st_copy.num_simplices() == st.num_simplices() - assert list(st_copy.get_filtration()) == list(st.get_filtration()) - - assert st.__is_persistence_defined() == True - assert st_copy.__is_persistence_defined() == False - - st_copy.assign_filtration([1, 2, 3], 2.) - assert list(st_copy.get_filtration()) == list(st.get_filtration()) - - # test double free - del st - del st_copy - def test_simplex_tree_deep_copy_constructor(): st = SimplexTree() st.insert([1, 2, 3], 0.) # persistence is not copied st.compute_persistence() - st_copy = SimplexTree(st, copy = True) + st_copy = SimplexTree(st) # TODO(VR): when #463 is merged, replace with # assert st_copy == st assert st_copy.num_vertices() == st.num_vertices() @@ -526,26 +503,3 @@ def test_simplex_tree_deep_copy_constructor(): # test double free del st del st_copy - -def test_simplex_tree_shallow_copy(): - st = SimplexTree() - st.insert([1, 2, 3], 0.) - # persistence is not copied - st.compute_persistence() - - st_copy = SimplexTree(st, copy = False) - # TODO(VR): when #463 is merged, replace with - # assert st_copy == st - assert st_copy.num_vertices() == st.num_vertices() - assert st_copy.num_simplices() == st.num_simplices() - assert list(st_copy.get_filtration()) == list(st.get_filtration()) - - assert st.__is_persistence_defined() == True - assert st_copy.__is_persistence_defined() == False - - st_copy.assign_filtration([1, 2, 3], 2.) - assert list(st_copy.get_filtration()) == list(st.get_filtration()) - - # test double free - del st - del st_copy -- cgit v1.2.3 From c7303733b28d3bc429d4bdfd030409e07430599d Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Mon, 14 Feb 2022 11:29:49 +0100 Subject: Rewrite test as #463 is merged. Rephrase constructor documentation --- src/python/gudhi/simplex_tree.pyx | 4 ++-- src/python/test/test_simplex_tree.py | 20 ++++++++------------ 2 files changed, 10 insertions(+), 14 deletions(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index 6701d98d..a2b8719a 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -41,9 +41,9 @@ cdef class SimplexTree: def __init__(self, other = None): """SimplexTree constructor. - :param other: If `other` is a `None` (default value), an empty `SimplexTree` is created. + :param other: If `other` is `None` (default value), an empty `SimplexTree` is created. If `other` is a `SimplexTree`, the `SimplexTree` is constructed from a deep copy of `other`. - :type other: SimplexTree + :type other: SimplexTree (Optional) :returns: An empty or a copy simplex tree. :rtype: SimplexTree diff --git a/src/python/test/test_simplex_tree.py b/src/python/test/test_simplex_tree.py index 32fc63ec..2c2e09a2 100755 --- a/src/python/test/test_simplex_tree.py +++ b/src/python/test/test_simplex_tree.py @@ -463,20 +463,18 @@ def test_equality_operator(): def test_simplex_tree_deep_copy(): st = SimplexTree() st.insert([1, 2, 3], 0.) - # persistence is not copied + # compute persistence only on the original st.compute_persistence() st_copy = st.copy() - # TODO(VR): when #463 is merged, replace with - # assert st_copy == st - assert st_copy.num_vertices() == st.num_vertices() - assert st_copy.num_simplices() == st.num_simplices() + assert st_copy == st st_filt_list = list(st.get_filtration()) - assert list(st_copy.get_filtration()) == st_filt_list + # check persistence is not copied assert st.__is_persistence_defined() == True assert st_copy.__is_persistence_defined() == False + # remove something in the copy and check the copy is included in the original st_copy.remove_maximal_simplex([1, 2, 3]) a_filt_list = list(st_copy.get_filtration()) assert len(a_filt_list) < len(st_filt_list) @@ -491,20 +489,18 @@ def test_simplex_tree_deep_copy(): def test_simplex_tree_deep_copy_constructor(): st = SimplexTree() st.insert([1, 2, 3], 0.) - # persistence is not copied + # compute persistence only on the original st.compute_persistence() st_copy = SimplexTree(st) - # TODO(VR): when #463 is merged, replace with - # assert st_copy == st - assert st_copy.num_vertices() == st.num_vertices() - assert st_copy.num_simplices() == st.num_simplices() + assert st_copy == st st_filt_list = list(st.get_filtration()) - assert list(st_copy.get_filtration()) == st_filt_list + # check persistence is not copied assert st.__is_persistence_defined() == True assert st_copy.__is_persistence_defined() == False + # remove something in the copy and check the copy is included in the original st_copy.remove_maximal_simplex([1, 2, 3]) a_filt_list = list(st_copy.get_filtration()) assert len(a_filt_list) < len(st_filt_list) -- cgit v1.2.3 From 1a0551c033e55024cd0f00302cd9df1f356bab44 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Mon, 14 Feb 2022 11:31:53 +0100 Subject: some left over --- src/python/gudhi/simplex_tree.pyx | 1 - 1 file changed, 1 deletion(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index a2b8719a..8f760422 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -62,7 +62,6 @@ cdef class SimplexTree: def __dealloc__(self): cdef Simplex_tree_interface_full_featured* ptr = self.get_ptr() - # Avoid double free - The original is in charge of deletion if ptr != NULL: del ptr if self.pcohptr != NULL: -- cgit v1.2.3 From d5ac245a6dc4ab2d6e30689fc5d95503c40b6187 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Mon, 14 Feb 2022 12:31:30 +0100 Subject: code review: ctor shall raise a TypeError when constructed from something else than a SimplexTree --- src/python/gudhi/simplex_tree.pyx | 10 +++++++--- src/python/test/test_simplex_tree.py | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index 8f760422..e1685ded 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -47,6 +47,7 @@ cdef class SimplexTree: :returns: An empty or a copy simplex tree. :rtype: SimplexTree + :raises TypeError: In case `other` is neither `None`, nor a `SimplexTree`. :note: If the `SimplexTree` is a copy, it requires :func:`compute_persistence` to be launched again as the persistence result is not copied. """ @@ -54,9 +55,12 @@ cdef class SimplexTree: # The real cython constructor def __cinit__(self, other = None): cdef SimplexTree ostr - if other and type(other) is SimplexTree: - ostr = other - self.thisptr = (new Simplex_tree_interface_full_featured(dereference(ostr.get_ptr()))) + if other: + if type(other) is SimplexTree: + ostr = other + self.thisptr = (new Simplex_tree_interface_full_featured(dereference(ostr.get_ptr()))) + else: + raise TypeError("`other` argument requires to be of type `SimplexTree`, or `None`.") else: self.thisptr = (new Simplex_tree_interface_full_featured()) diff --git a/src/python/test/test_simplex_tree.py b/src/python/test/test_simplex_tree.py index 2c2e09a2..a8180ce8 100755 --- a/src/python/test/test_simplex_tree.py +++ b/src/python/test/test_simplex_tree.py @@ -511,3 +511,7 @@ def test_simplex_tree_deep_copy_constructor(): # test double free del st del st_copy + +def test_simplex_tree_constructor_exception(): + with pytest.raises(TypeError): + st = SimplexTree(other = "Construction from a string shall raise an exception") -- cgit v1.2.3 From 4e63e52758daa7ef27782b4f129b7e55fa73de43 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Mon, 28 Feb 2022 09:50:08 +0100 Subject: code review: use 'isinstance' instead of 'type' --- src/python/gudhi/simplex_tree.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index e1685ded..711796d4 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -56,7 +56,7 @@ cdef class SimplexTree: def __cinit__(self, other = None): cdef SimplexTree ostr if other: - if type(other) is SimplexTree: + if isinstance(other, SimplexTree): ostr = other self.thisptr = (new Simplex_tree_interface_full_featured(dereference(ostr.get_ptr()))) else: -- cgit v1.2.3 From e7ba8967e9b5c3c7260522003d8f87e643b7912e Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Mon, 28 Feb 2022 10:56:47 +0100 Subject: doc review: reformulate the copy notes --- src/python/gudhi/simplex_tree.pyx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index 711796d4..91e079aa 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -48,8 +48,8 @@ cdef class SimplexTree: :rtype: SimplexTree :raises TypeError: In case `other` is neither `None`, nor a `SimplexTree`. - :note: If the `SimplexTree` is a copy, it requires :func:`compute_persistence` to be launched again as the - persistence result is not copied. + :note: If the `SimplexTree` is a copy, the persistence information is not copied. If you need it in the clone, + you have to call :func:`compute_persistence` on it even if you had already computed it in the original. """ # The real cython constructor @@ -86,7 +86,8 @@ cdef class SimplexTree: :returns: A simplex tree that is a deep copy of itself. :rtype: SimplexTree - :note: copy requires :func:`compute_persistence` to be launched again as the persistence result is not copied. + :note: The persistence information is not copied. If you need it in the clone, you have to call + :func:`compute_persistence` on it even if you had already computed it in the original. """ stree = SimplexTree() cdef Simplex_tree_interface_full_featured* stree_ptr -- cgit v1.2.3 From 41a976cc85ab36dc2df748b9d6900d77e76b2fa7 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Mon, 28 Feb 2022 14:58:24 +0100 Subject: code review: copy simplex tree c++ pointer in a factorized nogil function --- src/python/gudhi/simplex_tree.pyx | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index 91e079aa..b8fabf78 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -54,11 +54,9 @@ cdef class SimplexTree: # The real cython constructor def __cinit__(self, other = None): - cdef SimplexTree ostr if other: if isinstance(other, SimplexTree): - ostr = other - self.thisptr = (new Simplex_tree_interface_full_featured(dereference(ostr.get_ptr()))) + self.thisptr = _get_copy_intptr(other) else: raise TypeError("`other` argument requires to be of type `SimplexTree`, or `None`.") else: @@ -90,12 +88,7 @@ cdef class SimplexTree: :func:`compute_persistence` on it even if you had already computed it in the original. """ stree = SimplexTree() - cdef Simplex_tree_interface_full_featured* stree_ptr - cdef Simplex_tree_interface_full_featured* self_ptr=self.get_ptr() - with nogil: - stree_ptr = new Simplex_tree_interface_full_featured(dereference(self_ptr)) - - stree.thisptr = (stree_ptr) + stree.thisptr = _get_copy_intptr(self) return stree def __deepcopy__(self): @@ -687,3 +680,6 @@ cdef class SimplexTree: :rtype: bool """ return dereference(self.get_ptr()) == dereference(other.get_ptr()) + +cdef intptr_t _get_copy_intptr(SimplexTree stree) nogil: + return (new Simplex_tree_interface_full_featured(dereference(stree.get_ptr()))) -- cgit v1.2.3 From 924302f092d6d08c59500c5f6e75bcad9416581a Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Mon, 28 Mar 2022 08:39:21 +0200 Subject: doc review: remove segfault note as it is no more the case --- src/python/gudhi/simplex_tree.pyx | 3 --- 1 file changed, 3 deletions(-) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index 644110c1..fc704358 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -487,9 +487,6 @@ cdef class SimplexTree: with this new simplex (represented as a list of int). If `blocker_func` returns `True`, the simplex is removed, otherwise it is kept. The algorithm then proceeds with the next candidate. - Note that you cannot update the filtration value of the simplex during the evaluation of `blocker_func`, as it - would segfault. - :param max_dim: Expansion maximal dimension value. :type max_dim: int :param blocker_func: Blocker oracle. -- cgit v1.2.3 From f6a45247e9a9f126c214d1b1003ae19fb2cc84a3 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Mon, 28 Mar 2022 09:00:23 +0200 Subject: doc review: add a warning about phantom simplices for blocker_func --- src/python/gudhi/simplex_tree.pyx | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/python/gudhi/simplex_tree.pyx') diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index fc704358..a4914184 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -487,6 +487,11 @@ cdef class SimplexTree: with this new simplex (represented as a list of int). If `blocker_func` returns `True`, the simplex is removed, otherwise it is kept. The algorithm then proceeds with the next candidate. + .. warning:: + Several candidates of the same dimension may be inserted simultaneously before calling `block_simplex`, so + if you examine the complex in `block_simplex`, you may hit a few simplices of the same dimension that have + not been vetted by `block_simplex` yet, or have already been rejected but not yet removed. + :param max_dim: Expansion maximal dimension value. :type max_dim: int :param blocker_func: Blocker oracle. -- cgit v1.2.3