From 4ae1d7f83d28609c88e9be7d27223b950a3b84ee Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Fri, 22 Sep 2023 17:31:31 +0200 Subject: [PATCH 1/3] Fix fast cofaces for insert_batch_vertices and add a corresponding unitary test --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 18 +++++++++++++++++- .../test/simplex_tree_unit_test.cpp | 18 ++++++++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 39231f83b1..af4eedadd4 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -1317,10 +1317,26 @@ class Simplex_tree { * already present, its filtration value is not modified, unlike with other insertion functions. */ template void insert_batch_vertices(VertexRange const& vertices, Filtration_value filt = 0) { + // When Options::link_nodes_by_label, update_simplex_tree_after_node_insertion must be called only if + // not already inserted - copy vertex handles from flat_map + std::vector old_members(root_.members().size()); + if constexpr (Options::link_nodes_by_label) { + for (auto sh = root_.members().begin(); sh != root_.members().end(); sh++) + old_members.emplace_back(sh->first); + } + auto verts = vertices | boost::adaptors::transformed([&](auto v){ - return Dit_value_t(v, Node(&root_, filt)); }); + return Dit_value_t(v, Node(&root_, filt)); }); root_.members_.insert(boost::begin(verts), boost::end(verts)); if (dimension_ < 0 && !root_.members_.empty()) dimension_ = 0; + for (auto sh = root_.members().begin(); sh != root_.members().end(); sh++) { + if constexpr (Options::link_nodes_by_label) { + if (std::find(old_members.begin(), old_members.end(), sh->first) == std::end(old_members)) + update_simplex_tree_after_node_insertion(sh); + } else { + update_simplex_tree_after_node_insertion(sh); + } + } } /** \brief Expands the Simplex_tree containing only its one skeleton diff --git a/src/Simplex_tree/test/simplex_tree_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_unit_test.cpp index 8612a659d5..fa1a444151 100644 --- a/src/Simplex_tree/test/simplex_tree_unit_test.cpp +++ b/src/Simplex_tree/test/simplex_tree_unit_test.cpp @@ -1156,8 +1156,12 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_tree_boundaries_and_opposite_vertex_iterat } } -BOOST_AUTO_TEST_CASE(batch_vertices) { - typedef Simplex_tree<> typeST; +typedef boost::mpl::list, + Simplex_tree, + Simplex_tree > + list_of_tested_variants_wo_fast_persistence; + +BOOST_AUTO_TEST_CASE_TEMPLATE(batch_vertices, typeST, list_of_tested_variants_wo_fast_persistence) { std::clog << "********************************************************************" << std::endl; std::clog << "TEST BATCH VERTEX INSERTION" << std::endl; typeST st; @@ -1168,6 +1172,16 @@ BOOST_AUTO_TEST_CASE(batch_vertices) { BOOST_CHECK(st.num_simplices() == 4); BOOST_CHECK(st.filtration(st.find({2})) == 0.); BOOST_CHECK(st.filtration(st.find({3})) == 1.5); + + for (auto coface : st.star_simplex_range(st.find({5}))) { + std::clog << "coface"; + for (auto vertex : st.simplex_vertex_range(coface)) { + std::clog << " " << vertex; + // Should be the only one + BOOST_CHECK(vertex == 5); + } + std::clog << "\n"; + } } BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_tree_clear, typeST, list_of_tested_variants) { From 6ca067ceaee03f14d83c009d25976856da0cd5d5 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Mon, 25 Sep 2023 10:58:13 +0200 Subject: [PATCH 2/3] code review: Another approach to fix insert_batch_vertices with fast_cofaces --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index af4eedadd4..d2828688f6 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -1317,24 +1317,14 @@ class Simplex_tree { * already present, its filtration value is not modified, unlike with other insertion functions. */ template void insert_batch_vertices(VertexRange const& vertices, Filtration_value filt = 0) { - // When Options::link_nodes_by_label, update_simplex_tree_after_node_insertion must be called only if - // not already inserted - copy vertex handles from flat_map - std::vector old_members(root_.members().size()); - if constexpr (Options::link_nodes_by_label) { - for (auto sh = root_.members().begin(); sh != root_.members().end(); sh++) - old_members.emplace_back(sh->first); - } - auto verts = vertices | boost::adaptors::transformed([&](auto v){ - return Dit_value_t(v, Node(&root_, filt)); }); + return Dit_value_t(v, Node(&root_, filt)); }); root_.members_.insert(boost::begin(verts), boost::end(verts)); if (dimension_ < 0 && !root_.members_.empty()) dimension_ = 0; - for (auto sh = root_.members().begin(); sh != root_.members().end(); sh++) { - if constexpr (Options::link_nodes_by_label) { - if (std::find(old_members.begin(), old_members.end(), sh->first) == std::end(old_members)) + if constexpr (Options::link_nodes_by_label) { + for (auto sh = root_.members().begin(); sh != root_.members().end(); sh++) { + if (nodes_label_to_list_.find(sh->first) == nodes_label_to_list_.end()) update_simplex_tree_after_node_insertion(sh); - } else { - update_simplex_tree_after_node_insertion(sh); } } } From 13baadbac94789487e0a259676e2fe6bd86ce063 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Mon, 25 Sep 2023 17:04:25 +0200 Subject: [PATCH 3/3] code review: update newly inserted simplex (the one that are not linked) --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index d2828688f6..9f93672d63 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -1323,7 +1323,8 @@ class Simplex_tree { if (dimension_ < 0 && !root_.members_.empty()) dimension_ = 0; if constexpr (Options::link_nodes_by_label) { for (auto sh = root_.members().begin(); sh != root_.members().end(); sh++) { - if (nodes_label_to_list_.find(sh->first) == nodes_label_to_list_.end()) + // update newly inserted simplex (the one that are not linked) + if (!sh->second.list_max_vertex_hook_.is_linked()) update_simplex_tree_after_node_insertion(sh); } }