Fix of duplicated vertices in list.

Modify dimension computation.


git-svn-id: svn+ssh://scm.gforge.inria.fr/svnroot/gudhi/branches/simplex_tree_insert_duplicated_vertices_fix_vincent@4107 636b058d-ea47-450e-bf9e-a15bfbe3eedb

Former-commit-id: e6ac9ee4c9d34e59c07d9f5b16b13b5151608009

2 files changed, 85 insertions, 14 deletions

diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h
index 4b18651c..6bb42037 100644
--- a/src/Simplex_tree/include/gudhi/Simplex_tree.h
+++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h
@@ -787,19 +787,22 @@ class Simplex_tree {
  private:
   /// Same as insert_simplex_and_subfaces but assumes that the range of vertices is sorted
   template<class ForwardVertexRange = std::initializer_list<Vertex_handle>>
-  std::pair<Simplex_handle, bool> insert_simplex_and_subfaces_sorted(const ForwardVertexRange& Nsimplex, Filtration_value filt = 0) {
+  std::pair<Simplex_handle, bool> insert_simplex_and_subfaces_sorted(const ForwardVertexRange& Nsimplex,
+                                                                     Filtration_value filt = 0) {
     auto first = std::begin(Nsimplex);
     auto last = std::end(Nsimplex);
     if (first == last)
       return { null_simplex(), true }; // FIXME: false would make more sense to me.
     GUDHI_CHECK(std::is_sorted(first, last), "simplex vertices listed in unsorted order");
-    // Update dimension if needed. We could wait to see if the insertion succeeds, but I doubt there is much to gain.
-    dimension_ = (std::max)(dimension_, static_cast<int>(std::distance(first, last)) - 1);
-    return rec_insert_simplex_and_subfaces_sorted(root(), first, last, filt);
+    return rec_insert_simplex_and_subfaces_sorted(root(), first, last, filt, 0);
   }
   // To insert {1,2,3,4}, we insert {2,3,4} twice, once at the root, and once below 1.
   template<class ForwardVertexIterator>
-  std::pair<Simplex_handle, bool> rec_insert_simplex_and_subfaces_sorted(Siblings* sib, ForwardVertexIterator first, ForwardVertexIterator last, Filtration_value filt) {
+  std::pair<Simplex_handle, bool> rec_insert_simplex_and_subfaces_sorted(Siblings* sib,
+                                                                         ForwardVertexIterator first,
+                                                                         ForwardVertexIterator last,
+                                                                         Filtration_value filt,
+                                                                         int dimension) {
     // An alternative strategy would be:
     // - try to find the complete simplex, if found (and low filtration) exit
     // - insert all the vertices at once in sib
@@ -811,19 +814,30 @@ class Simplex_tree {
     bool one_is_new = insertion_result.second;
     if (!one_is_new) {
       if (filtration(simplex_one) > filt) {
-	assign_filtration(simplex_one, filt);
+        assign_filtration(simplex_one, filt);
       } else {
-	// FIXME: this interface makes no sense, and it doesn't seem to be tested.
-	insertion_result.first = null_simplex();
+        // FIXME: this interface makes no sense, and it doesn't seem to be tested.
+        insertion_result.first = null_simplex();
       }
     }
-    if (++first == last) return insertion_result;
+    // Next iteration to avoid consecutive equal values
+    while ((vertex_one == *first) && (first < last)) {
+      ++first;
+    }
+    // End of insertion
+    if (first == last) {
+      dimension_ = (std::max)(dimension_, dimension);
+      return insertion_result;
+    }
     if (!has_children(simplex_one))
       // TODO: have special code here, we know we are building the whole subtree from scratch.
       simplex_one->second.assign_children(new Siblings(sib, vertex_one));
-    auto res = rec_insert_simplex_and_subfaces_sorted(simplex_one->second.children(), first, last, filt);
+    ++dimension;
+    auto res = rec_insert_simplex_and_subfaces_sorted(simplex_one->second.children(), first, last, filt, dimension);
     // No need to continue if the full simplex was already there with a low enough filtration value.
-    if (res.first != null_simplex()) rec_insert_simplex_and_subfaces_sorted(sib, first, last, filt);
+    if (res.first != null_simplex()) {
+      rec_insert_simplex_and_subfaces_sorted(sib, first, last, filt, dimension);
+    }
     return res;
   }
 
diff --git a/src/Simplex_tree/test/simplex_tree_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_unit_test.cpp
index b27bbce8..f2a5d54d 100644
--- a/src/Simplex_tree/test/simplex_tree_unit_test.cpp
+++ b/src/Simplex_tree/test/simplex_tree_unit_test.cpp
@@ -2,10 +2,11 @@
 #include <fstream>
 #include <string>
 #include <algorithm>
-#include <utility> // std::pair, std::make_pair
-#include <cmath> // float comparison
+#include <utility>  // std::pair, std::make_pair
+#include <cmath>  // float comparison
 #include <limits>
-#include <functional> // greater
+#include <functional>  // greater
+#include <tuple>  // std::tie
 
 #define BOOST_TEST_DYN_LINK
 #define BOOST_TEST_MODULE "simplex_tree"
@@ -921,3 +922,59 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_tree_insert_graph, Graph, list_of_graph_va
 
   BOOST_CHECK(st1 == st2);
 }
+
+BOOST_AUTO_TEST_CASE_TEMPLATE(insert_duplicated_vertices, typeST, list_of_tested_variants) {
+  std::cout << "********************************************************************" << std::endl;
+  std::cout << "TEST INSERT DUPLICATED VERTICES" << std::endl;
+  typeST st;
+
+  typename typeST::Simplex_handle sh;
+  bool success = false;
+  std::tie(sh, success) = st.insert_simplex_and_subfaces({1});
+  BOOST_CHECK(success);
+  BOOST_CHECK(sh != st.null_simplex());
+  std::cout << "st.dimension(sh)= " << st.dimension(sh) << std::endl;
+  BOOST_CHECK(st.dimension(sh) == 0);
+  std::tie(sh, success) = st.insert_simplex_and_subfaces({2, 2});
+  BOOST_CHECK(success);
+  BOOST_CHECK(sh != st.null_simplex());
+  std::cout << "st.dimension(sh)= " << st.dimension(sh) << std::endl;
+  BOOST_CHECK(st.dimension(sh) == 0);
+  std::tie(sh, success) = st.insert_simplex_and_subfaces({3, 3, 3});
+  BOOST_CHECK(success);
+  BOOST_CHECK(sh != st.null_simplex());
+  std::cout << "st.dimension(sh)= " << st.dimension(sh) << std::endl;
+  BOOST_CHECK(st.dimension(sh) == 0);
+  std::tie(sh, success) = st.insert_simplex_and_subfaces({4, 4, 4, 4});
+  BOOST_CHECK(success);
+  BOOST_CHECK(sh != st.null_simplex());
+  std::cout << "st.dimension(sh)= " << st.dimension(sh) << std::endl;
+  BOOST_CHECK(st.dimension(sh) == 0);
+
+  std::cout << "dimension =" << st.dimension() << " - num_vertices = " << st.num_vertices()
+            << " - num_simplices = " << st.num_simplices() << std::endl;
+  BOOST_CHECK(st.dimension() == 0);
+  BOOST_CHECK(st.num_simplices() == st.num_vertices());
+
+  std::tie(sh, success) = st.insert_simplex_and_subfaces({2, 1, 1, 2});
+  BOOST_CHECK(success);
+  BOOST_CHECK(sh != st.null_simplex());
+  std::cout << "st.dimension(sh)= " << st.dimension(sh) << std::endl;
+  BOOST_CHECK(st.dimension(sh) == 1);
+
+  std::cout << "dimension =" << st.dimension() << " - num_vertices = " << st.num_vertices()
+            << " - num_simplices = " << st.num_simplices() << std::endl;
+  BOOST_CHECK(st.dimension() == 1);
+  BOOST_CHECK(st.num_simplices() == st.num_vertices() + 1);
+
+  // Already inserted
+  std::tie(sh, success) = st.insert_simplex_and_subfaces({1, 2, 2, 1});
+  BOOST_CHECK(!success);
+  BOOST_CHECK(sh == st.null_simplex());
+
+  std::cout << "dimension =" << st.dimension() << " - num_vertices = " << st.num_vertices()
+            << " - num_simplices = " << st.num_simplices() << std::endl;
+  BOOST_CHECK(st.dimension() == 1);
+  BOOST_CHECK(st.num_simplices() == st.num_vertices() + 1);
+
+}