From af065fc6d2d4771485118bb5122b3d5203b367ed Mon Sep 17 00:00:00 2001 From: vrouvrea Date: Fri, 24 Aug 2018 07:39:49 +0000 Subject: Fix move constructor and assignment for the Simplex tree git-svn-id: svn+ssh://scm.gforge.inria.fr/svnroot/gudhi/branches/simplex_tree_fix_vincent@3831 636b058d-ea47-450e-bf9e-a15bfbe3eedb Former-commit-id: ff520472b6a1b9f00d0688fbe7dc467af50e16fe --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 64 ++++++++++++++++------ .../test/simplex_tree_ctor_and_move_unit_test.cpp | 59 ++++++++++++++++---- 2 files changed, 94 insertions(+), 29 deletions(-) (limited to 'src/Simplex_tree') diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 4759b352..8e0ddf75 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -298,11 +298,11 @@ class Simplex_tree { /** \brief User-defined copy constructor reproduces the whole tree structure. */ Simplex_tree(const Simplex_tree& simplex_source) : null_vertex_(simplex_source.null_vertex_), - root_(nullptr, null_vertex_ , simplex_source.root_.members_), + root_(nullptr, null_vertex_, simplex_source.root_.members_), filtration_vect_(), dimension_(simplex_source.dimension_) { #ifdef DEBUG_TRACES - std::cout << "copy constructor" << std::endl; + std::cout << "Simplex_tree copy constructor" << std::endl; #endif // DEBUG_TRACES auto root_source = simplex_source.root_; rec_copy(&root_, &root_source); @@ -323,17 +323,29 @@ class Simplex_tree { } } - /** \brief User-defined move constructor moves the whole tree structure. */ - Simplex_tree(Simplex_tree && old) - : null_vertex_(std::move(old.null_vertex_)), - root_(std::move(old.root_)), - filtration_vect_(std::move(old.filtration_vect_)), - dimension_(std::move(old.dimension_)) { + /** \brief User-defined move constructor relocates the whole tree structure. + * \exception std::invalid_argument In debug mode, if the simplex_source is invalid. + */ + Simplex_tree(Simplex_tree && simplex_source) + : null_vertex_(std::move(simplex_source.null_vertex_)), + root_(std::move(simplex_source.root_)), + filtration_vect_(std::move(simplex_source.filtration_vect_)), + dimension_(std::move(simplex_source.dimension_)) { #ifdef DEBUG_TRACES - std::cout << "move constructor" << std::endl; + std::cout << "Simplex_tree move constructor" << std::endl; #endif // DEBUG_TRACES - old.dimension_ = -1; - old.root_ = Siblings(nullptr, null_vertex_); + for (auto& map_el : root_.members()) { + if (map_el.second.children()->oncles() == &(simplex_source.root_)) + // reset with the moved root_ pointer value + map_el.second.children()->oncles_ = &root_; + else + // if simplex is of dimension 0, oncles_ shall be nullptr + GUDHI_CHECK(map_el.second.children()->oncles_ == nullptr, + std::invalid_argument("Simplex_tree move constructor from an invalid Simplex_tree")); + } + // just need to set dimension_ on source to make it available again + // (filtration_vect_ and members are already set from the move) + simplex_source.dimension_ = -1; } /** \brief Destructor; deallocates the whole tree structure. */ @@ -351,16 +363,24 @@ class Simplex_tree { #ifdef DEBUG_TRACES std::cout << "copy assignment" << std::endl; #endif // DEBUG_TRACES - this->null_vertex_ = simplex_source.null_vertex_; - root_ = Siblings(nullptr, null_vertex_ , simplex_source.root_.members_); - this->filtration_vect_.clear(); - this->dimension_ = simplex_source.dimension_; + null_vertex_ = simplex_source.null_vertex_; + filtration_vect_.clear(); + dimension_ = simplex_source.dimension_; auto root_source = simplex_source.root_; - rec_copy(&(this->root_), &root_source); + // Here a copy will be done + root_ = Siblings(nullptr, null_vertex_); + root_.members() = Dictionary(boost::container::ordered_unique_range, root_source.members().begin(), root_source.members().end()); + // Needs to reassign children + for (auto& map_el : root_.members()) { + map_el.second.assign_children(&root_); + } + rec_copy(&root_, &root_source); return *this; } - /** \brief User-defined move assignment reproduces the whole tree structure. */ + /** \brief User-defined move assignment relocates the whole tree structure. + * \exception std::invalid_argument In debug mode, if the simplex_source is invalid. + */ Simplex_tree& operator=(Simplex_tree&& simplex_source) { #ifdef DEBUG_TRACES @@ -372,6 +392,16 @@ class Simplex_tree { std::swap( root_, simplex_source.root_ ); std::swap( filtration_vect_, simplex_source.filtration_vect_ ); std::swap( dimension_, simplex_source.dimension_ ); + + for (auto& map_el : root_.members()) { + if (map_el.second.children()->oncles() == &(simplex_source.root_)) + // reset with the moved root_ pointer value + map_el.second.children()->oncles_ = &root_; + else + // if simplex is of dimension 0, oncles_ shall be nullptr + GUDHI_CHECK(map_el.second.children()->oncles_ == nullptr, + std::invalid_argument("Simplex_tree move constructor from an invalid Simplex_tree")); + } } return *this; } diff --git a/src/Simplex_tree/test/simplex_tree_ctor_and_move_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_ctor_and_move_unit_test.cpp index 15eaf612..95e99afe 100644 --- a/src/Simplex_tree/test/simplex_tree_ctor_and_move_unit_test.cpp +++ b/src/Simplex_tree/test/simplex_tree_ctor_and_move_unit_test.cpp @@ -1,5 +1,6 @@ #include #include +#include #define BOOST_TEST_DYN_LINK #define BOOST_TEST_MODULE "simplex_tree_constructor_and_move" @@ -14,38 +15,58 @@ using namespace Gudhi; typedef boost::mpl::list, Simplex_tree> list_of_tested_variants; -template -SimplicialComplex move_it(SimplicialComplex sc) { - return sc; -} +template +void print_simplex_filtration(Simplex_tree& st, const std::string& msg) { + // Required before browsing through filtration values + st.initialize_filtration(); + + std::cout << "********************************************************************\n"; + std::cout << "* " << msg << "\n"; + std::cout << "* The complex contains " << st.num_simplices() << " simplices"; + std::cout << " - dimension " << st.dimension() << "\n"; + std::cout << "* Iterator on Simplices in the filtration, with [filtration value]:\n"; + for (auto f_simplex : st.filtration_simplex_range()) { + std::cout << " " + << "[" << st.filtration(f_simplex) << "] "; + for (auto vertex : st.simplex_vertex_range(f_simplex)) std::cout << "(" << vertex << ")"; + std::cout << std::endl; + } +} BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_copy_constructor, Simplex_tree, list_of_tested_variants) { - std::cout << "********************************************************************" << std::endl; - std::cout << "TEST OF COPY CONSTRUCTOR" << std::endl; Simplex_tree st; st.insert_simplex_and_subfaces({2, 1, 0}, 3.0); st.insert_simplex_and_subfaces({0, 1, 6, 7}, 4.0); st.insert_simplex_and_subfaces({3, 0}, 2.0); st.insert_simplex_and_subfaces({3, 4, 5}, 3.0); + st.insert_simplex_and_subfaces({8}, 1.0); /* Inserted simplex: */ /* 1 6 */ /* o---o */ /* /X\7/ */ - /* o---o---o---o */ - /* 2 0 3\X/4 */ + /* o---o---o---o o */ + /* 2 0 3\X/4 8 */ /* o */ /* 5 */ /* */ /* In other words: */ - /* A facet [2,1,0] */ - /* An edge [0,3] */ - /* A facet [3,4,5] */ - /* A cell [0,1,6,7] */ + /* A facet [2,1,0] */ + /* An edge [0,3] */ + /* A facet [3,4,5] */ + /* A cell [0,1,6,7] */ + /* A vertex [8] */ + + print_simplex_filtration(st, "Default Simplex_tree is initialized"); + + std::cout << "********************************************************************" << std::endl; + std::cout << "TEST OF COPY CONSTRUCTOR" << std::endl; Simplex_tree st1(st); Simplex_tree st2(st); + print_simplex_filtration(st1, "First copy constructor from the default Simplex_tree"); + print_simplex_filtration(st2, "Second copy constructor from the default Simplex_tree"); // Cross check BOOST_CHECK(st1 == st2); BOOST_CHECK(st == st2); @@ -55,8 +76,11 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_copy_constructor, Simplex_tree, list_of_te std::cout << "TEST OF COPY ASSIGNMENT" << std::endl; Simplex_tree st3; st3 = st; + print_simplex_filtration(st3, "First copy assignment from the default Simplex_tree"); Simplex_tree st4; st4 = st; + print_simplex_filtration(st4, "Second copy assignment from the default Simplex_tree"); + // Cross check BOOST_CHECK(st3 == st4); BOOST_CHECK(st == st4); @@ -65,18 +89,29 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_copy_constructor, Simplex_tree, list_of_te std::cout << "********************************************************************" << std::endl; std::cout << "TEST OF MOVE CONSTRUCTOR" << std::endl; Simplex_tree st5(std::move(st1)); + print_simplex_filtration(st5, "First move constructor from the default Simplex_tree"); + print_simplex_filtration(st1, "First moved Simplex_tree shall be empty"); Simplex_tree st6(std::move(st2)); + print_simplex_filtration(st6, "Second move constructor from the default Simplex_tree"); + print_simplex_filtration(st2, "Second moved Simplex_tree shall be empty"); // Cross check BOOST_CHECK(st5 == st6); BOOST_CHECK(st == st6); BOOST_CHECK(st5 == st); + Simplex_tree empty_st; + BOOST_CHECK(st1 == st2); + BOOST_CHECK(empty_st == st2); + BOOST_CHECK(st1 == empty_st); + std::cout << "********************************************************************" << std::endl; std::cout << "TEST OF MOVE ASSIGNMENT" << std::endl; // A swap is a copy ctor of a tmp value, then it uses move assignment std::swap(st3, st4); + print_simplex_filtration(st3, "First move assignment from the default Simplex_tree"); + print_simplex_filtration(st4, "Second move assignment from the default Simplex_tree"); BOOST_CHECK(st3 == st4); BOOST_CHECK(st == st4); BOOST_CHECK(st3 == st); -- cgit v1.2.3