From 73c80013156a7f94b6ed7222da986a99e8c70fab Mon Sep 17 00:00:00 2001 From: Marc Glisse Date: Thu, 5 Nov 2020 13:48:17 +0100 Subject: [PATCH 1/3] Test insertion of simplex with NaN filtration value --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 8 ++++++-- src/python/gudhi/simplex_tree.pyx | 10 ++++++++++ src/python/test/test_simplex_tree.py | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 85d6c3b0db..8aa9fcfc1d 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -763,9 +763,11 @@ class Simplex_tree { return insert_vertex_vector(copy, filtration); } - /** \brief Insert a N-simplex and all his subfaces, from a N-simplex represented by a range of + /** \brief Insert a N-simplex and all its subfaces, from a N-simplex represented by a range of * Vertex_handles, in the simplicial complex. * + * Inserting a simplex with filtration value NaN does not modify the filtration value of any pre-existing simplex. + * * @param[in] Nsimplex range of Vertex_handles, representing the vertices of the new N-simplex * @param[in] filtration the filtration value assigned to the new N-simplex. * @return If the new simplex is inserted successfully (i.e. it was not in the @@ -1352,7 +1354,8 @@ class Simplex_tree { * filtration values. * @return True if any filtration value was modified, false if the filtration was already non-decreasing. * - * If a simplex has a `NaN` filtration value, it is considered lower than any other defined filtration value. + * If a simplex of dimension 1 or more has a `NaN` filtration value, + * it is considered lower than any other defined filtration value. */ bool make_filtration_non_decreasing() { bool modified = false; @@ -1379,6 +1382,7 @@ class Simplex_tree { for (auto& simplex : boost::adaptors::reverse(sib->members())) { // Find the maximum filtration value in the border Boundary_simplex_range boundary = boundary_simplex_range(&simplex); + // We would need a loop using fmax to get consistent results when a vertex has NaN. Boundary_simplex_iterator max_border = std::max_element(std::begin(boundary), std::end(boundary), [](Simplex_handle sh1, Simplex_handle sh2) { return filtration(sh1) < filtration(sh2); diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index 813dc5c20b..a22fc5114a 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -184,6 +184,11 @@ cdef class SimplexTree: simplices are already present with a higher filtration value, their filtration value is lowered. + .. note:: + + Inserting a simplex with filtration value `math.nan` does not + modify the filtration value of any simplex already present. + :param simplex: The N-simplex to insert, represented by a list of vertex. :type simplex: list of int @@ -368,6 +373,11 @@ cdef class SimplexTree: """This function ensures that each simplex has a higher filtration value than its faces by increasing the filtration values. + .. note:: + + For this function, a value of NaN for a simplex of dimension 1 or more + is considered smaller than any other value. + :returns: True if any filtration value was modified, False if the filtration was already non-decreasing. :rtype: bool diff --git a/src/python/test/test_simplex_tree.py b/src/python/test/test_simplex_tree.py index 3b23fa0b56..60f27d37d4 100755 --- a/src/python/test/test_simplex_tree.py +++ b/src/python/test/test_simplex_tree.py @@ -10,6 +10,7 @@ from gudhi import SimplexTree import pytest +import math __author__ = "Vincent Rouvreau" __copyright__ = "Copyright (C) 2016 Inria" @@ -399,3 +400,17 @@ def test_boundaries_iterator(): with pytest.raises(RuntimeError): list(st.get_boundaries([6])) # (6) does not exist + +# insert with NaN must not modify filtration values +# make_filtration_non_decreasing considers NaN small +def test_nan(): + st = SimplexTree() + st.insert([1], 2.) + st.insert([4], 5.) + st.insert([2], -1.) + st.insert([1, 2, 4], math.nan) + assert math.isnan(st.filtration([2, 4])) + st.make_filtration_non_decreasing() + assert st.filtration([1, 2, 4]) == 5. + assert st.filtration([1, 2]) == 2. + assert st.filtration([2]) == -1. From 52a54061902d584c2780ef961707666ed1064171 Mon Sep 17 00:00:00 2001 From: Marc Glisse Date: Thu, 5 Nov 2020 18:00:50 +0100 Subject: [PATCH 2/3] Slightler longer test --- src/python/test/test_simplex_tree.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/python/test/test_simplex_tree.py b/src/python/test/test_simplex_tree.py index 60f27d37d4..7fd257715e 100755 --- a/src/python/test/test_simplex_tree.py +++ b/src/python/test/test_simplex_tree.py @@ -408,9 +408,16 @@ def test_nan(): st.insert([1], 2.) st.insert([4], 5.) st.insert([2], -1.) + st.insert([1, 4], 6.) st.insert([1, 2, 4], math.nan) + assert st.filtration([1]) == 2. + assert st.filtration([2]) == -1. + assert st.filtration([4]) == 5. + assert st.filtration([1, 4]) == 6. assert math.isnan(st.filtration([2, 4])) st.make_filtration_non_decreasing() - assert st.filtration([1, 2, 4]) == 5. + assert st.filtration([1, 2, 4]) == 6. assert st.filtration([1, 2]) == 2. + assert st.filtration([1, 4]) == 6. assert st.filtration([2]) == -1. + assert st.filtration([4]) == 5. From 767f8cb31b548c57b86945631ee06014668f3e38 Mon Sep 17 00:00:00 2001 From: Marc Glisse Date: Thu, 5 Nov 2020 20:44:43 +0100 Subject: [PATCH 3/3] More doc. --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 6 +++++- src/python/gudhi/simplex_tree.pyx | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 8aa9fcfc1d..551176c85e 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -766,7 +766,9 @@ class Simplex_tree { /** \brief Insert a N-simplex and all its subfaces, from a N-simplex represented by a range of * Vertex_handles, in the simplicial complex. * - * Inserting a simplex with filtration value NaN does not modify the filtration value of any pre-existing simplex. + * Inserting a simplex with filtration value NaN does not modify the + * filtration value of any pre-existing simplex. Pre-existing NaN may or may + * not be updated by an insertion with a real value. * * @param[in] Nsimplex range of Vertex_handles, representing the vertices of the new N-simplex * @param[in] filtration the filtration value assigned to the new N-simplex. @@ -821,6 +823,8 @@ class Simplex_tree { Simplex_handle simplex_one = insertion_result.first; bool one_is_new = insertion_result.second; if (!one_is_new) { + // In order for an insertion to handle pre-existing NaN like +Inf, we would need + // filtration(simplex_one) > filt || (isnan(filtration(simplex_one)) && ! isnan(filt)) if (filtration(simplex_one) > filt) { assign_filtration(simplex_one, filt); } else { diff --git a/src/python/gudhi/simplex_tree.pyx b/src/python/gudhi/simplex_tree.pyx index a22fc5114a..1d1e6ba7e9 100644 --- a/src/python/gudhi/simplex_tree.pyx +++ b/src/python/gudhi/simplex_tree.pyx @@ -188,6 +188,10 @@ cdef class SimplexTree: Inserting a simplex with filtration value `math.nan` does not modify the filtration value of any simplex already present. + However, it produces an invalid filtration, which needs to be fixed + with :meth:`make_filtration_non_decreasing` or + :meth:`assign_filtration`. Inserting simplices with real filtration + value may or may not update pre-existing NaN. :param simplex: The N-simplex to insert, represented by a list of vertex.