From 250757df365dc3b2c238e23414a365b78bfffe82 Mon Sep 17 00:00:00 2001 From: hschreiber Date: Fri, 2 Aug 2024 19:43:42 +0200 Subject: [PATCH 01/23] begining --- .../include/gudhi/Persistent_cohomology.h | 12 +- src/Simplex_tree/include/gudhi/Simplex_tree.h | 240 ++++++++++++------ .../Simplex_tree/Simplex_tree_iterators.h | 30 +-- .../Simplex_tree_node_explicit_storage.h | 3 + .../Simplex_tree/Simplex_tree_siblings.h | 15 ++ .../Simplex_tree_star_simplex_iterators.h | 2 +- .../test/simplex_tree_unit_test.cpp | 26 ++ 7 files changed, 229 insertions(+), 99 deletions(-) diff --git a/src/Persistent_cohomology/include/gudhi/Persistent_cohomology.h b/src/Persistent_cohomology/include/gudhi/Persistent_cohomology.h index 47364edfc3..03d686f10c 100644 --- a/src/Persistent_cohomology/include/gudhi/Persistent_cohomology.h +++ b/src/Persistent_cohomology/include/gudhi/Persistent_cohomology.h @@ -559,7 +559,7 @@ class Persistent_cohomology { void output_diagram(std::ostream& ostream = std::cout) { cmp_intervals_by_length cmp(cpx_); std::sort(std::begin(persistent_pairs_), std::end(persistent_pairs_), cmp); - for (auto pair : persistent_pairs_) { + for (const auto& pair : persistent_pairs_) { ostream << get<2>(pair) << " " << cpx_->dimension(get<0>(pair)) << " " << cpx_->filtration(get<0>(pair)) << " " << cpx_->filtration(get<1>(pair)) << " " << std::endl; @@ -571,7 +571,7 @@ class Persistent_cohomology { diagram_out.exceptions(diagram_out.failbit); cmp_intervals_by_length cmp(cpx_); std::sort(std::begin(persistent_pairs_), std::end(persistent_pairs_), cmp); - for (auto pair : persistent_pairs_) { + for (const auto& pair : persistent_pairs_) { diagram_out << cpx_->dimension(get<0>(pair)) << " " << cpx_->filtration(get<0>(pair)) << " " << cpx_->filtration(get<1>(pair)) << std::endl; @@ -586,7 +586,7 @@ class Persistent_cohomology { // size for an empty complex std::vector betti_numbers(std::max(dim_max_, 0)); - for (auto pair : persistent_pairs_) { + for (const auto& pair : persistent_pairs_) { // Count never ended persistence intervals if (cpx_->null_simplex() == get<1>(pair)) { // Increment corresponding betti number @@ -604,7 +604,7 @@ class Persistent_cohomology { int betti_number(int dimension) const { int betti_number = 0; - for (auto pair : persistent_pairs_) { + for (const auto& pair : persistent_pairs_) { // Count never ended persistence intervals if (cpx_->null_simplex() == get<1>(pair)) { if (cpx_->dimension(get<0>(pair)) == dimension) { @@ -625,7 +625,7 @@ class Persistent_cohomology { // Init Betti numbers vector with zeros until Simplicial complex dimension and don't allocate a vector of negative // size for an empty complex std::vector betti_numbers(std::max(dim_max_, 0)); - for (auto pair : persistent_pairs_) { + for (const auto& pair : persistent_pairs_) { // Count persistence intervals that covers the given interval // null_simplex test : if the function is called with to=+infinity, we still get something useful. And it will // still work if we change the complex filtration function to reject null simplices. @@ -647,7 +647,7 @@ class Persistent_cohomology { int persistent_betti_number(int dimension, Filtration_value from, Filtration_value to) const { int betti_number = 0; - for (auto pair : persistent_pairs_) { + for (const auto& pair : persistent_pairs_) { // Count persistence intervals that covers the given interval // null_simplex test : if the function is called with to=+infinity, we still get something useful. And it will // still work if we change the complex filtration function to reject null simplices. diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index c9d3a243f0..84836ccd6e 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -145,22 +145,40 @@ class Simplex_tree { Filtration_value minval; Filtration_value maxval; Extended_filtration_data(){} - Extended_filtration_data(Filtration_value vmin, Filtration_value vmax): minval(vmin), maxval(vmax) {} + Extended_filtration_data(const Filtration_value& vmin, const Filtration_value& vmax): minval(vmin), maxval(vmax) {} }; typedef typename std::conditional::type Key_simplex_base; + using Filtration_value_rep = std::conditional_t, + Filtration_value&, Filtration_value>; + struct Filtration_simplex_base_real { Filtration_simplex_base_real() : filt_(0) {} - void assign_filtration(Filtration_value f) { filt_ = f; } - Filtration_value filtration() const { return filt_; } + void assign_filtration(const Filtration_value& f) { filt_ = f; } + const Filtration_value_rep filtration() const { return filt_; } + Filtration_value_rep filtration() { return filt_; } + + constexpr static Filtration_value_rep get_infinity(){ + if constexpr (!std::is_arithmetic_v) inf_ = _inf(); + return inf_; + } + private: Filtration_value filt_; + static constexpr Filtration_value _inf() { + return std::numeric_limits::has_infinity ? std::numeric_limits::infinity() + : std::numeric_limits::max(); + } + static constexpr Filtration_value inf_ = _inf(); }; struct Filtration_simplex_base_dummy { Filtration_simplex_base_dummy() {} - void assign_filtration(Filtration_value GUDHI_CHECK_code(f)) { GUDHI_CHECK(f == 0, "filtration value specified for a complex that does not store them"); } - Filtration_value filtration() const { return 0; } + void assign_filtration(const Filtration_value& GUDHI_CHECK_code(f)) { + GUDHI_CHECK(f == 0, "filtration value specified for a complex that does not store them"); + } + Filtration_value filtration() const { return Filtration_value(0); } + Filtration_value filtration() { return Filtration_value(0); } }; typedef typename std::conditional::type Filtration_simplex_base; @@ -172,10 +190,11 @@ class Simplex_tree { * They are essentially pointers into internal vectors, and any insertion or removal * of a simplex may invalidate any other Simplex_handle in the complex, * unless Options::stable_simplex_handles == true. */ - typedef typename Dictionary::iterator Simplex_handle; + typedef typename Dictionary::const_iterator Simplex_handle; private: typedef typename Dictionary::iterator Dictionary_it; + typedef typename Dictionary::const_iterator Const_dictionary_it; typedef typename Dictionary_it::value_type Dit_value_t; struct return_first { @@ -290,6 +309,11 @@ class Simplex_tree { /** \brief Returns a range over the vertices of the simplicial complex. * The order is increasing according to < on Vertex_handles.*/ + Complex_vertex_range complex_vertex_range() const { + return Complex_vertex_range( + boost::make_transform_iterator(root_.members_.begin(), return_first()), + boost::make_transform_iterator(root_.members_.end(), return_first())); + } Complex_vertex_range complex_vertex_range() { return Complex_vertex_range( boost::make_transform_iterator(root_.members_.begin(), return_first()), @@ -301,6 +325,10 @@ class Simplex_tree { * In the Simplex_tree, the tree is traverse in a depth-first fashion. * Consequently, simplices are ordered according to lexicographic order on the list of * Vertex_handles of a simplex, read in increasing < order for Vertex_handles. */ + Complex_simplex_range complex_simplex_range() const { + return Complex_simplex_range(Complex_simplex_iterator(this), + Complex_simplex_iterator()); + } Complex_simplex_range complex_simplex_range() { return Complex_simplex_range(Complex_simplex_iterator(this), Complex_simplex_iterator()); @@ -315,6 +343,10 @@ class Simplex_tree { * * The simplices are ordered according to lexicographic order on the list of * Vertex_handles of a simplex, read in increasing < order for Vertex_handles. */ + Skeleton_simplex_range skeleton_simplex_range(int dim) const { + return Skeleton_simplex_range(Skeleton_simplex_iterator(this, dim), + Skeleton_simplex_iterator()); + } Skeleton_simplex_range skeleton_simplex_range(int dim) { return Skeleton_simplex_range(Skeleton_simplex_iterator(this, dim), Skeleton_simplex_iterator()); @@ -339,11 +371,17 @@ class Simplex_tree { maybe_initialize_filtration(); return filtration_vect_; } + Filtration_simplex_range const& filtration_simplex_range(Indexing_tag = Indexing_tag()) const { + GUDHI_CHECK(!filtration_vect_.empty() || Complex_simplex_range().empty(), + "Filtration range was not yet initialized. Please call `initialize_filtration`."); + if (filtration_vect_.empty() && !Complex_simplex_range().empty()) throw std::invalid_argument("filtration_simplex_range should be non const"); + return filtration_vect_; + } /** \brief Returns a range over the vertices of a simplex. * * The order in which the vertices are visited is the decreasing order for < on Vertex_handles, - * which is consequenlty + * which is consequently * equal to \f$(-1)^{\text{dim} \sigma}\f$ the canonical orientation on the simplex. */ Simplex_vertex_range simplex_vertex_range(Simplex_handle sh) const { @@ -371,6 +409,11 @@ class Simplex_tree { return Boundary_simplex_range(Boundary_simplex_iterator(this, sh), Boundary_simplex_iterator(this)); } + template + Boundary_simplex_range boundary_simplex_range(SimplexHandle sh) const { + return Boundary_simplex_range(Boundary_simplex_iterator(this, sh), + Boundary_simplex_iterator(this)); + } /** \brief Given a simplex, returns a range over the simplices of its boundary and their opposite vertices. * @@ -388,6 +431,11 @@ class Simplex_tree { return Boundary_opposite_vertex_simplex_range(Boundary_opposite_vertex_simplex_iterator(this, sh), Boundary_opposite_vertex_simplex_iterator(this)); } + template + Boundary_opposite_vertex_simplex_range boundary_opposite_vertex_simplex_range(SimplexHandle sh) const { + return Boundary_opposite_vertex_simplex_range(Boundary_opposite_vertex_simplex_iterator(this, sh), + Boundary_opposite_vertex_simplex_iterator(this)); + } /** @} */ // end range and iterator methods /** \name Constructor/Destructor @@ -544,7 +592,7 @@ class Simplex_tree { /** \brief Checks if two simplex trees are equal. */ template - bool operator==(Simplex_tree& st2) { + bool operator==(Simplex_tree& st2) const { if ((null_vertex_ != st2.null_vertex_) || (dimension_ != st2.dimension_ && !dimension_to_be_lowered_ && !st2.dimension_to_be_lowered_)) return false; @@ -553,14 +601,14 @@ class Simplex_tree { /** \brief Checks if two simplex trees are different. */ template - bool operator!=(Simplex_tree& st2) { + bool operator!=(Simplex_tree& st2) const { return (!(*this == st2)); } private: /** rec_equal: Checks recursively whether or not two simplex trees are equal, using depth first search. */ template - bool rec_equal(Siblings* s1, OtherSiblings* s2) { + bool rec_equal(Siblings const* s1, OtherSiblings const* s2) const { if (s1->members().size() != s2->members().size()) return false; auto sh2 = s2->members().begin(); @@ -583,11 +631,15 @@ class Simplex_tree { * * Same as `filtration()`, but does not handle `null_simplex()`. */ - static Filtration_value filtration_(Simplex_handle sh) { + static const Filtration_value_rep filtration_(Simplex_handle sh) { GUDHI_CHECK (sh != null_simplex(), "null simplex"); return sh->second.filtration(); } + Dictionary_it _to_node_it(Simplex_handle sh){ + return const_cast(self_siblings(sh))->to_non_const_it(sh); + } + public: /** \brief Returns the key associated to a simplex. * @@ -611,21 +663,40 @@ class Simplex_tree { * Called on the null_simplex, it returns infinity. * If SimplexTreeOptions::store_filtration is false, returns 0. */ - static Filtration_value filtration(Simplex_handle sh) { + Filtration_value_rep filtration(Simplex_handle sh) { + if (sh == null_simplex()) { + // get_infinity rewrites the inf_ value in case someone changes the value of the reference + //the only way to avoid this would be to forbid the use of this method with null simplices: + + // if constexpr (Options::store_filtration && !std::is_arithmetic_v) + // throw std::invalid_argument( + // "Simplex handle does not redirect to a real simplex and a non constant reference cannot be returned. " + // "Please use the `const` version of the method."); + // else + return Filtration_simplex_base_real::get_infinity(); + } + + return sh->second.filtration(); + } + const Filtration_value_rep filtration(Simplex_handle sh) const { if (sh != null_simplex()) { return sh->second.filtration(); } else { - return std::numeric_limits::infinity(); + return Filtration_simplex_base_real::get_infinity(); } } + static bool has_filtration_value_infinity(Simplex_handle sh){ + return sh->second.filtration() == Filtration_simplex_base_real::get_infinity(); + } + /** \brief Sets the filtration value of a simplex. * \exception std::invalid_argument In debug mode, if sh is a null_simplex. */ - void assign_filtration(Simplex_handle sh, Filtration_value fv) { + void assign_filtration(Simplex_handle sh, const Filtration_value& fv) { GUDHI_CHECK(sh != null_simplex(), std::invalid_argument("Simplex_tree::assign_filtration - cannot assign filtration on null_simplex")); - sh->second.assign_filtration(fv); + _to_node_it(sh)->second.assign_filtration(fv); } /** \brief Returns a Simplex_handle different from all Simplex_handles @@ -661,13 +732,13 @@ class Simplex_tree { /** \brief Returns the number of simplices in the simplex_tree. * * This function takes time linear in the number of simplices. */ - size_t num_simplices() { + size_t num_simplices() const { return num_simplices(root()); } private: /** \brief Returns the number of simplices in the simplex_tree. */ - size_t num_simplices(Siblings * sib) { + size_t num_simplices(const Siblings * sib) const { auto sib_begin = sib->members().begin(); auto sib_end = sib->members().end(); size_t simplices_number = sib->members().size(); @@ -685,7 +756,7 @@ class Simplex_tree { * @param curr_sib Pointer to the sibling container. * @return Height of the siblings in the tree (root counts as zero to make the height correspond to the dimension). */ - int dimension(Siblings* curr_sib) { + int dimension(Siblings const* curr_sib) const { int dim = -1; while (curr_sib != nullptr) { ++dim; @@ -717,7 +788,7 @@ class Simplex_tree { /** \brief Returns the dimension of a simplex. * * Must be different from null_simplex().*/ - int dimension(Simplex_handle sh) { + int dimension(Simplex_handle sh) const { return dimension(self_siblings(sh)); } @@ -730,7 +801,7 @@ class Simplex_tree { \details This function is not constant time because it can recompute dimension if required (can be triggered by `remove_maximal_simplex()` or `prune_above_filtration()`). */ - int dimension() { + int dimension() const { if (dimension_to_be_lowered_) lower_upper_bound_dimension(); return dimension_; @@ -764,7 +835,7 @@ class Simplex_tree { * on which we can call std::begin() function */ template> - Simplex_handle find(const InputVertexRange & s) { + Simplex_handle find(const InputVertexRange & s) const { auto first = std::begin(s); auto last = std::end(s); @@ -779,9 +850,9 @@ class Simplex_tree { private: /** Find function, with a sorted range of vertices. */ - Simplex_handle find_simplex(const std::vector & simplex) { - Siblings * tmp_sib = &root_; - Dictionary_it tmp_dit; + Simplex_handle find_simplex(const std::vector & simplex) const { + Siblings const* tmp_sib = &root_; + Const_dictionary_it tmp_dit; auto vi = simplex.begin(); if constexpr (Options::contiguous_vertices && !Options::stable_simplex_handles) { // Equivalent to the first iteration of the normal loop @@ -810,7 +881,7 @@ class Simplex_tree { /** \brief Returns the Simplex_handle corresponding to the 0-simplex * representing the vertex with Vertex_handle v. */ - Simplex_handle find_vertex(Vertex_handle v) { + Simplex_handle find_vertex(Vertex_handle v) const { if constexpr (Options::contiguous_vertices && !Options::stable_simplex_handles) { assert(contiguous_vertices()); return root_.members_.begin() + v; @@ -845,9 +916,9 @@ class Simplex_tree { */ template > std::pair insert_simplex_raw(const RandomVertexHandleRange& simplex, - Filtration_value filtration) { + const Filtration_value& filtration) { Siblings * curr_sib = &root_; - std::pair res_insert; + std::pair res_insert; auto vi = simplex.begin(); for (; vi != std::prev(simplex.end()); ++vi) { GUDHI_CHECK(*vi != null_vertex(), "cannot use the dummy null_vertex() as a real vertex"); @@ -911,7 +982,7 @@ class Simplex_tree { * .end() return input iterators, with 'value_type' Vertex_handle. */ template> std::pair insert_simplex(const InputVertexRange & simplex, - Filtration_value filtration = 0) { + const Filtration_value& filtration = 0) { auto first = std::begin(simplex); auto last = std::end(simplex); @@ -940,7 +1011,7 @@ class Simplex_tree { */ template> std::pair insert_simplex_and_subfaces(const InputVertexRange& Nsimplex, - Filtration_value filtration = 0) { + const Filtration_value& filtration = 0) { auto first = std::begin(Nsimplex); auto last = std::end(Nsimplex); @@ -969,7 +1040,7 @@ class Simplex_tree { std::pair rec_insert_simplex_and_subfaces_sorted(Siblings* sib, ForwardVertexIterator first, ForwardVertexIterator last, - Filtration_value filt) { + const Filtration_value& filt) { // 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 @@ -977,20 +1048,21 @@ class Simplex_tree { Vertex_handle vertex_one = *first; auto&& dict = sib->members(); auto insertion_result = dict.emplace(vertex_one, Node(sib, filt)); + std::pair result(insertion_result); // update extra data structures in the insertion is successful if (insertion_result.second) { // Only required when insertion is successful update_simplex_tree_after_node_insertion(insertion_result.first); } - Simplex_handle simplex_one = insertion_result.first; + auto simplex_one = insertion_result.first; bool one_is_new = insertion_result.second; if (!one_is_new) { if (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(); + result.first = null_simplex(); } } if (++first == last) return insertion_result; @@ -1013,14 +1085,14 @@ class Simplex_tree { /** Returns the two Simplex_handle corresponding to the endpoints of * and edge. sh must point to a 1-dimensional simplex. This is an * optimized version of the boundary computation. */ - std::pair endpoints(Simplex_handle sh) { + std::pair endpoints(Simplex_handle sh) const { assert(dimension(sh) == 1); return { find_vertex(sh->first), find_vertex(self_siblings(sh)->parent()) }; } /** Returns the Siblings containing a simplex.*/ template - static Siblings* self_siblings(SimplexHandle sh) { + static Siblings const* self_siblings(SimplexHandle sh) { if (sh->second.children()->parent() == sh->first) return sh->second.children()->oncles(); else @@ -1028,8 +1100,10 @@ class Simplex_tree { } public: + Siblings* root() { return &root_; } + /** Returns a pointer to the root nodes of the simplex tree. */ - Siblings * root() { + const Siblings * root() const { return &root_; } @@ -1108,7 +1182,7 @@ class Simplex_tree { * If the vertices list is empty, we need to check if curr_nbVertices matches with the dimension of the cofaces asked. */ void rec_coface(std::vector &vertices, Siblings *curr_sib, int curr_nbVertices, - std::vector& cofaces, bool star, int nbVertices) { + std::vector& cofaces, bool star, int nbVertices) const { if (!(star || curr_nbVertices <= nbVertices)) // dimension of actual simplex <= nbVertices return; for (Simplex_handle simplex = curr_sib->members().begin(); simplex != curr_sib->members().end(); ++simplex) { @@ -1159,6 +1233,9 @@ class Simplex_tree { Cofaces_simplex_range star_simplex_range(const Simplex_handle simplex) { return cofaces_simplex_range(simplex, 0); } + Cofaces_simplex_range star_simplex_range(const Simplex_handle simplex) const { + return cofaces_simplex_range(simplex, 0); + } /** \brief Compute the cofaces of a n simplex * \param simplex represent the n-simplex of which we search the n+codimension cofaces @@ -1171,6 +1248,15 @@ class Simplex_tree { * SimplexTreeOptions::link_nodes_by_label is true. */ Cofaces_simplex_range cofaces_simplex_range(const Simplex_handle simplex, int codimension) { + return _cofaces_simplex_range(simplex, codimension); + } + + Cofaces_simplex_range cofaces_simplex_range(const Simplex_handle simplex, int codimension) const{ + return _cofaces_simplex_range(simplex, codimension); + } + + private: + Cofaces_simplex_range _cofaces_simplex_range(const Simplex_handle simplex, int codimension) { // codimension must be positive or null integer assert(codimension >= 0); @@ -1199,7 +1285,6 @@ class Simplex_tree { } } - private: /** \brief Returns true iff the list of vertices of sh1 * is smaller than the list of vertices of sh2 w.r.t. * lexicographic order on the lists read in reverse. @@ -1207,7 +1292,7 @@ class Simplex_tree { * It defines a StrictWeakOrdering on simplices. The Simplex_vertex_iterators * must traverse the Vertex_handle in decreasing order. Reverse lexicographic order satisfy * the property that a subsimplex of a simplex is always strictly smaller with this order. */ - bool reverse_lexicographic_order(Simplex_handle sh1, Simplex_handle sh2) { + bool reverse_lexicographic_order(Simplex_handle sh1, Simplex_handle sh2) const { Simplex_vertex_range rg1 = simplex_vertex_range(sh1); Simplex_vertex_range rg2 = simplex_vertex_range(sh2); Simplex_vertex_iterator it1 = rg1.begin(); @@ -1314,7 +1399,7 @@ class Simplex_tree { // Should we actually forbid multiple edges? That would be consistent // with rejecting self-loops. if (v < u) std::swap(u, v); - auto sh = find_vertex(u); + auto sh = _to_node_it(find_vertex(u)); if (!has_children(sh)) { sh->second.assign_children(new Siblings(&root_, sh->first)); } @@ -1333,7 +1418,7 @@ class Simplex_tree { * The complex does not need to be empty before calling this function. However, if a vertex is * 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) { + void insert_batch_vertices(VertexRange const& vertices, const Filtration_value& filt = 0) { auto verts = vertices | boost::adaptors::transformed([&](auto v){ return Dit_value_t(v, Node(&root_, filt)); }); root_.members_.insert(boost::begin(verts), boost::end(verts)); @@ -1407,7 +1492,7 @@ class Simplex_tree { */ void insert_edge_as_flag( Vertex_handle u , Vertex_handle v - , Filtration_value fil + , const Filtration_value& fil , int dim_max , std::vector& added_simplices) { @@ -1480,7 +1565,7 @@ class Simplex_tree { { Node& node_u = static_cast(node_as_hook); //corresponding node, has label u Simplex_handle sh_u = simplex_handle_from_node(node_u); - Siblings * sib_u = self_siblings(sh_u); + Siblings const* sib_u = self_siblings(sh_u); if (sib_u->members().find(v) != sib_u->members().end()) { //v is the label of a sibling of node_u int curr_dim = dimension(sib_u); if (dim_max == -1 || curr_dim < dim_max){ @@ -1519,10 +1604,10 @@ class Simplex_tree { * 2- All Node in the members of sib, with label @p x and @p x < @p v, * need in turn a local_expansion by @p v iff N^+(x) contains @p v. */ - void compute_punctual_expansion( Vertex_handle v - , Siblings * sib - , Filtration_value fil - , int k //k == dim_max - dimension simplices in sib + void compute_punctual_expansion( Vertex_handle v + , Siblings * sib + , const Filtration_value& fil + , int k //k == dim_max - dimension simplices in sib , std::vector& added_simplices ) { //insertion always succeeds because the edge {u,v} used to not be here. auto res_ins_v = sib->members().emplace(v, Node(sib,fil)); @@ -1568,10 +1653,10 @@ class Simplex_tree { * k must be > 0 */ void create_local_expansion( - Simplex_handle sh_v //Node with label v which has just been inserted - , Siblings * curr_sib //Siblings containing the node sh_v - , Filtration_value fil_uv //Fil value of the edge uv in the zz filtration - , int k //Stopping condition for recursion based on max dim + Simplex_handle sh_v //Node with label v which has just been inserted + , Siblings * curr_sib //Siblings containing the node sh_v + , const Filtration_value& fil_uv //Fil value of the edge uv in the zz filtration + , int k //Stopping condition for recursion based on max dim , std::vector &added_simplices) //range of all new simplices { //pick N^+(v) //intersect N^+(v) with labels y > v in curr_sib @@ -1595,9 +1680,9 @@ class Simplex_tree { * Only called in the case of `void insert_edge_as_flag(...)`. */ void siblings_expansion( - Siblings * siblings // must contain elements - , Filtration_value fil - , int k // == max_dim expansion - dimension curr siblings + Siblings * siblings // must contain elements + , const Filtration_value& fil + , int k // == max_dim expansion - dimension curr siblings , std::vector & added_simplices ) { if (dimension_ > k) { @@ -1640,7 +1725,7 @@ class Simplex_tree { void create_expansion(Siblings * siblings, Dictionary_it& s_h, Dictionary_it& next, - Filtration_value fil, + const Filtration_value& fil, int k, std::vector* added_simplices = nullptr) { @@ -1687,7 +1772,7 @@ class Simplex_tree { static void intersection(std::vector >& intersection, Dictionary_it begin1, Dictionary_it end1, Dictionary_it begin2, Dictionary_it end2, - Filtration_value filtration_) { + const Filtration_value& filtration_) { if (begin1 == end1 || begin2 == end2) return; // ----->> while (true) { @@ -1836,7 +1921,7 @@ class Simplex_tree { * dim idx_1 ... idx_k fil where dim is the dimension of the simplex, * idx_1 ... idx_k are the row index (starting from 0) of the simplices of the boundary * of the simplex, and fil is its filtration value. */ - void print_hasse(std::ostream& os) { + void print_hasse(std::ostream& os) const { os << num_simplices() << " " << std::endl; for (auto sh : filtration_simplex_range()) { os << dimension(sh) << " "; @@ -1900,7 +1985,7 @@ class Simplex_tree { // Find the maximum filtration value in the border Boundary_simplex_range&& boundary = boundary_simplex_range(sh); Boundary_simplex_iterator max_border = std::max_element(std::begin(boundary), std::end(boundary), - [](Simplex_handle sh1, Simplex_handle sh2) { + [this](Simplex_handle sh1, Simplex_handle sh2) { return filtration(sh1) < filtration(sh2); }); @@ -1910,7 +1995,7 @@ class Simplex_tree { if (!(sh->second.filtration() >= max_filt_border_value)) { // Store the filtration modification information modified = true; - sh->second.assign_filtration(max_filt_border_value); + _to_node_it(sh)->second.assign_filtration(max_filt_border_value); } }; // Loop must be from the end to the beginning, as higher dimension simplex are always on the left part of the tree @@ -1939,7 +2024,7 @@ class Simplex_tree { * than it was before. However, `upper_bound_dimension()` will return the old value, which remains a valid upper * bound. If you care, you can call `dimension()` to recompute the exact dimension. */ - bool prune_above_filtration(Filtration_value filtration) { + bool prune_above_filtration(const Filtration_value& filtration) { if (std::numeric_limits::has_infinity && filtration == std::numeric_limits::infinity()) return false; // ---->> bool modified = rec_prune_above_filtration(root(), filtration); @@ -1949,7 +2034,7 @@ class Simplex_tree { } private: - bool rec_prune_above_filtration(Siblings* sib, Filtration_value filt) { + bool rec_prune_above_filtration(Siblings* sib, const Filtration_value& filt) { auto&& list = sib->members(); bool modified = false; bool emptied = false; @@ -2052,7 +2137,7 @@ class Simplex_tree { * \pre Be sure the simplex tree has not a too low dimension value as the deep search stops when the former dimension * has been reached (cf. `upper_bound_dimension()` and `set_dimension()` methods). */ - bool lower_upper_bound_dimension() { + bool lower_upper_bound_dimension() const { // reset automatic detection to recompute dimension_to_be_lowered_ = false; int new_dimension = -1; @@ -2093,12 +2178,13 @@ class Simplex_tree { update_simplex_tree_before_node_removal(sh); // Simplex is a leaf, it means the child is the Siblings owning the leaf - Siblings* child = sh->second.children(); + Dictionary_it nodeIt = _to_node_it(sh); + Siblings* child = nodeIt->second.children(); if ((child->size() > 1) || (child == root())) { // Not alone, just remove it from members // Special case when child is the root of the simplex tree, just remove it from members - child->erase(sh); + child->erase(nodeIt); } else { // Sibling is emptied : must be deleted, and its parent must point on his own Sibling child->oncles()->members().at(child->parent()).assign_children(child->oncles()); @@ -2124,7 +2210,7 @@ class Simplex_tree { * @param[in] efd Structure containing the minimum and maximum values of the original filtration. This the output of `extend_filtration()`. * @return A pair containing the original filtration value of the simplex as well as the simplex type. */ - std::pair decode_extended_filtration(Filtration_value f, const Extended_filtration_data& efd){ + std::pair decode_extended_filtration(const Filtration_value& f, const Extended_filtration_data& efd) const{ std::pair p; Filtration_value minval = efd.minval; Filtration_value maxval = efd.maxval; @@ -2211,7 +2297,7 @@ class Simplex_tree { * * For a lower-star filtration built with `make_filtration_non_decreasing()`, this is a way to invert the process and find out which vertex had its filtration value propagated to `sh`. * If several vertices have the same filtration value, the one it returns is arbitrary. */ - Vertex_handle vertex_with_same_filtration(Simplex_handle sh) { + Vertex_handle vertex_with_same_filtration(Simplex_handle sh) const { auto filt = filtration_(sh); for(auto v : simplex_vertex_range(sh)) if(filtration_(find_vertex(v)) == filt) @@ -2225,7 +2311,7 @@ class Simplex_tree { * If several edges have the same filtration value, the one it returns is arbitrary. * * \pre `sh` must have dimension at least 1. */ - Simplex_handle edge_with_same_filtration(Simplex_handle sh) { + Simplex_handle edge_with_same_filtration(Simplex_handle sh) const { // See issue #251 for potential speed improvements. auto&& vertices = simplex_vertex_range(sh); // vertices in decreasing order auto end = std::end(vertices); @@ -2258,7 +2344,7 @@ class Simplex_tree { * * For a filtration built with `make_filtration_non_decreasing()`, this is a way to invert the process and find out which simplex had its filtration value propagated to `sh`. * If several minimal (for inclusion) simplices have the same filtration value, the one it returns is arbitrary, and it is not guaranteed to be the one with smallest dimension. */ - Simplex_handle minimal_simplex_with_same_filtration(Simplex_handle sh) { + Simplex_handle minimal_simplex_with_same_filtration(Simplex_handle sh) const { auto filt = filtration_(sh); // Naive implementation, it can be sped up. for(auto b : boundary_simplex_range(sh)) @@ -2286,7 +2372,7 @@ class Simplex_tree { // unordered_map Vertex_handle v -> list of all Nodes with label v. std::unordered_map nodes_label_to_list_; - List_max_vertex* nodes_by_label(Vertex_handle v) { + List_max_vertex* nodes_by_label(Vertex_handle v) const { if constexpr (Options::link_nodes_by_label) { auto it_v = nodes_label_to_list_.find(v); if (it_v != nodes_label_to_list_.end()) { @@ -2303,7 +2389,7 @@ class Simplex_tree { static Simplex_handle simplex_handle_from_node(Node& node) { if constexpr (Options::stable_simplex_handles){ //Relies on the Dictionary type to be boost::container::map. - //If the type changes or boost fondamentally changes something on the structure of their map, + //If the type changes or boost fundamentally changes something on the structure of their map, //a safer/more general but much slower version is: // if (node.children()->parent() == label) { // verifies if node is a leaf // return children->oncles()->find(label); @@ -2355,7 +2441,7 @@ class Simplex_tree { #endif // DEBUG_TRACES if constexpr (Options::link_nodes_by_label) { // Creates an entry with sh->first if not already in the map and insert sh->second at the end of the list - nodes_label_to_list_[sh->first].push_back(sh->second); + nodes_label_to_list_[sh->first].push_back(_to_node_it(sh)->second); } } @@ -2381,7 +2467,7 @@ class Simplex_tree { * @param[in] filt_value The new filtration value. * @param[in] min_dim The minimal dimension. Default value is 0. */ - void reset_filtration(Filtration_value filt_value, int min_dim = 0) { + void reset_filtration(const Filtration_value& filt_value, int min_dim = 0) { rec_reset_filtration(&root_, filt_value, min_dim); clear_filtration(); // Drop the cache. } @@ -2392,7 +2478,7 @@ class Simplex_tree { * @param[in] filt_value The new filtration value. * @param[in] min_depth The minimal depth. */ - void rec_reset_filtration(Siblings * sib, Filtration_value filt_value, int min_depth) { + void rec_reset_filtration(Siblings * sib, const Filtration_value& filt_value, int min_depth) { for (auto sh = sib->members().begin(); sh != sib->members().end(); ++sh) { if (min_depth <= 0) { sh->second.assign_filtration(filt_value); @@ -2411,7 +2497,7 @@ class Simplex_tree { * @warning It is meant to return the same size with the same SimplexTreeOptions and on a computer with the same * architecture. */ - std::size_t get_serialization_size() { + std::size_t get_serialization_size() const { const std::size_t vh_byte_size = sizeof(Vertex_handle); const std::size_t fv_byte_size = SimplexTreeOptions::store_filtration ? sizeof(Filtration_value) : 0; const std::size_t buffer_byte_size = vh_byte_size + num_simplices() * (fv_byte_size + 2 * vh_byte_size); @@ -2450,7 +2536,7 @@ class Simplex_tree { /* 0d(list of [c] children)00(number of [b,d] children)00(number of [d] children) */ /* Without explanation and with filtration values: */ /* 04 0a F(a) 0b F(b) 0c F(c) 0d F(d) 01 0b F(a,b) 00 02 0c F(b,c) 0d F(b,d) 01 0d F(b,c,d) 00 00 01 0d F(c,d) 00 00 */ - void serialize(char* buffer, const std::size_t buffer_size) { + void serialize(char* buffer, const std::size_t buffer_size) const { char* buffer_end = rec_serialize(&root_, buffer); if (static_cast(buffer_end - buffer) != buffer_size) throw std::invalid_argument("Serialization does not match end of buffer"); @@ -2458,7 +2544,7 @@ class Simplex_tree { private: /** \brief Serialize each element of the sibling and recursively call serialization. */ - char* rec_serialize(Siblings *sib, char* buffer) { + char* rec_serialize(Siblings *sib, char* buffer) const { char* ptr = buffer; ptr = Gudhi::simplex_tree::serialize_trivial(static_cast(sib->members().size()), ptr); #ifdef DEBUG_TRACES @@ -2556,8 +2642,8 @@ class Simplex_tree { /** \brief Simplices ordered according to a filtration.*/ std::vector filtration_vect_; /** \brief Upper bound on the dimension of the simplicial complex.*/ - int dimension_; - bool dimension_to_be_lowered_ = false; + mutable int dimension_; + mutable bool dimension_to_be_lowered_ = false; }; // Print a Simplex_tree in os. diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h index 71be462f1d..1591ebd01b 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h @@ -67,7 +67,7 @@ class Simplex_tree_simplex_vertex_iterator : public boost::iterator_facade< sib_ = sib_->oncles(); } - Siblings * sib_; + Siblings const* sib_; Vertex_handle v_; }; @@ -93,7 +93,7 @@ class Simplex_tree_boundary_simplex_iterator : public boost::iterator_facade< } // any end() iterator - explicit Simplex_tree_boundary_simplex_iterator(SimplexTree * st) + explicit Simplex_tree_boundary_simplex_iterator(SimplexTree const* st) : last_(st->null_vertex()), next_(st->null_vertex()), sib_(nullptr), @@ -102,7 +102,7 @@ class Simplex_tree_boundary_simplex_iterator : public boost::iterator_facade< } template - Simplex_tree_boundary_simplex_iterator(SimplexTree * st, SimplexHandle sh) + Simplex_tree_boundary_simplex_iterator(SimplexTree const* st, SimplexHandle sh) : last_(sh->first), next_(st->null_vertex()), sib_(nullptr), @@ -111,7 +111,7 @@ class Simplex_tree_boundary_simplex_iterator : public boost::iterator_facade< // Only check once at the beginning instead of for every increment, as this is expensive. if constexpr (SimplexTree::Options::contiguous_vertices) GUDHI_CHECK(st_->contiguous_vertices(), "The set of vertices is not { 0, ..., n } without holes"); - Siblings * sib = st->self_siblings(sh); + Siblings const* sib = st->self_siblings(sh); next_ = sib->parent(); sib_ = sib->oncles(); if (sib_ != nullptr) { @@ -146,8 +146,8 @@ class Simplex_tree_boundary_simplex_iterator : public boost::iterator_facade< return; } - Siblings * for_sib = sib_; - Siblings * new_sib = sib_->oncles(); + Siblings const* for_sib = sib_; + Siblings const* new_sib = sib_->oncles(); auto rit = suffix_.rbegin(); if constexpr (SimplexTree::Options::contiguous_vertices && !SimplexTree::Options::stable_simplex_handles) { if (new_sib == nullptr) { @@ -180,9 +180,9 @@ class Simplex_tree_boundary_simplex_iterator : public boost::iterator_facade< Vertex_handle last_; // last vertex of the simplex Vertex_handle next_; // next vertex to push in suffix_ Static_vertex_vector suffix_; - Siblings * sib_; // where the next search will start from + Siblings const* sib_; // where the next search will start from Simplex_handle sh_; // current Simplex_handle in the boundary - SimplexTree * st_; // simplex containing the simplicial complex + SimplexTree const* st_; // simplex containing the simplicial complex }; /** \brief Iterator over the simplices of the boundary of a simplex and their opposite vertices. @@ -224,7 +224,7 @@ class Simplex_tree_boundary_opposite_vertex_simplex_iterator : public boost::ite // Only check once at the beginning instead of for every increment, as this is expensive. if constexpr (SimplexTree::Options::contiguous_vertices) GUDHI_CHECK(st_->contiguous_vertices(), "The set of vertices is not { 0, ..., n } without holes"); - Siblings * sib = st->self_siblings(sh); + Siblings const* sib = st->self_siblings(sh); next_ = sib->parent(); sib_ = sib->oncles(); if (sib_ != nullptr) { @@ -294,9 +294,9 @@ class Simplex_tree_boundary_opposite_vertex_simplex_iterator : public boost::ite Vertex_handle last_; // last vertex of the simplex Vertex_handle next_; // next vertex to push in suffix_ Static_vertex_vector suffix_; - Siblings * sib_; // where the next search will start from + Siblings const* sib_; // where the next search will start from std::pair baov_; // a pair containing the current Simplex_handle in the boundary and its opposite vertex - SimplexTree * st_; // simplex containing the simplicial complex + SimplexTree const* st_; // simplex containing the simplicial complex }; /*---------------------------------------------------------------------------*/ @@ -369,8 +369,8 @@ class Simplex_tree_complex_simplex_iterator : public boost::iterator_facade< } Simplex_handle sh_; - Siblings * sib_; - SimplexTree * st_; + Siblings const* sib_; + SimplexTree const* st_; }; /** \brief Iterator over the simplices of the skeleton of a given @@ -450,8 +450,8 @@ class Simplex_tree_skeleton_simplex_iterator : public boost::iterator_facade< } Simplex_handle sh_; - Siblings * sib_; - SimplexTree * st_; + Siblings const* sib_; + SimplexTree const* st_; int dim_skel_; int curr_dim_; }; diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_node_explicit_storage.h b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_node_explicit_storage.h index d7dbb541be..417fe844dc 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_node_explicit_storage.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_node_explicit_storage.h @@ -57,6 +57,9 @@ struct GUDHI_EMPTY_BASE_CLASS_OPTIMIZATION Simplex_tree_node_explicit_storage : Siblings * children() { return children_; } + const Siblings * children() const { + return children_; + } private: Siblings * children_; diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_siblings.h b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_siblings.h index d849eeba07..c259eb72bc 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_siblings.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_siblings.h @@ -43,6 +43,7 @@ class Simplex_tree_siblings { typedef typename SimplexTree::Node Node; typedef MapContainer Dictionary; typedef typename MapContainer::iterator Dictionary_it; + typedef typename MapContainer::const_iterator Const_dictionary_it; /* Default constructor.*/ Simplex_tree_siblings() @@ -87,10 +88,16 @@ class Simplex_tree_siblings { Dictionary_it find(Vertex_handle v) { return members_.find(v); } + Const_dictionary_it find(Vertex_handle v) const { + return members_.find(v); + } Simplex_tree_siblings * oncles() { return oncles_; } + const Simplex_tree_siblings * oncles() const { + return oncles_; + } Vertex_handle parent() const { return parent_; @@ -100,6 +107,10 @@ class Simplex_tree_siblings { return members_; } + const Dictionary & members() const { + return members_; + } + size_t size() const { return members_.size(); } @@ -108,6 +119,10 @@ class Simplex_tree_siblings { members_.erase(iterator); } + Dictionary_it to_non_const_it(Const_dictionary_it it) { + return members_.erase(it, it); + } + Simplex_tree_siblings * oncles_; Vertex_handle parent_; Dictionary members_; diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_star_simplex_iterators.h b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_star_simplex_iterators.h index 1298223f6e..316ca31cb5 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_star_simplex_iterators.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_star_simplex_iterators.h @@ -265,7 +265,7 @@ class Simplex_tree_optimized_star_simplex_iterator // curr Simplex_handle, returned by operator*, pointing to a coface of s Simplex_handle sh_; // set of siblings containing sh_ in the Simplex_tree - Siblings* sib_; // + Siblings const* sib_; // // Save children in a list to avoid calling sib_->members().find(.) std::vector children_stack_; // true iff sh_ points to the root of a coface subtree diff --git a/src/Simplex_tree/test/simplex_tree_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_unit_test.cpp index 5b133ca225..ba314e9f97 100644 --- a/src/Simplex_tree/test/simplex_tree_unit_test.cpp +++ b/src/Simplex_tree/test/simplex_tree_unit_test.cpp @@ -48,6 +48,32 @@ typedef boost::mpl::list, Simplex_tree, Simplex_tree > list_of_tested_variants; +BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_tree_filtrations, typeST, list_of_tested_variants) { + std::clog << "********************************************************************" << std::endl; + std::clog << "TEST OF ..." << std::endl; + typeST st; + + st.insert_simplex({0}, 0); + st.insert_simplex({1}, 1); + st.insert_simplex({0,1}, 2); + st.insert_simplex({2}, 3); + + auto res1 = st.find({0}); + + BOOST_CHECK(st.filtration(res1) == 0); + + if constexpr (typeST::Options::store_filtration && !std::is_arithmetic_v){ + typename typeST::Filtration_value& fil = st.filtration(res1); + fil = 5; + + BOOST_CHECK(st.filtration(res1) == 5); + } + + BOOST_CHECK(st.filtration(st.find({0,1})) == 2); + + BOOST_CHECK(st.has_filtration_value_infinity(st.null_simplex())); +} + template void test_empty_simplex_tree(typeST& tst) { typedef typename typeST::Vertex_handle Vertex_handle; From 5ac7d5878d156169cd045fca065c80228f707e02 Mon Sep 17 00:00:00 2001 From: hschreiber Date: Mon, 12 Aug 2024 17:10:44 +0200 Subject: [PATCH 02/23] const compiles --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 64 +++++++++++-------- .../Simplex_tree/Simplex_tree_iterators.h | 12 ++-- .../Simplex_tree_star_simplex_iterators.h | 26 ++++---- 3 files changed, 58 insertions(+), 44 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 84836ccd6e..b71d2cbc15 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -196,6 +196,7 @@ class Simplex_tree { typedef typename Dictionary::iterator Dictionary_it; typedef typename Dictionary::const_iterator Const_dictionary_it; typedef typename Dictionary_it::value_type Dit_value_t; + typedef typename Const_dictionary_it::value_type Const_dit_value_t; struct return_first { Vertex_handle operator()(const Dit_value_t& p_sh) const { @@ -687,6 +688,7 @@ class Simplex_tree { } static bool has_filtration_value_infinity(Simplex_handle sh){ + if (sh == null_simplex()) return true; return sh->second.filtration() == Filtration_simplex_base_real::get_infinity(); } @@ -704,7 +706,7 @@ class Simplex_tree { * * One can call filtration(null_simplex()). */ static Simplex_handle null_simplex() { - return Dictionary_it(); + return Const_dictionary_it(); } /** \brief Returns a fixed number not in the interval [0, `num_simplices()`). */ @@ -821,7 +823,7 @@ class Simplex_tree { /** \brief Returns the children of the node in the simplex tree pointed by sh. * \exception std::invalid_argument In debug mode, if sh has no child. */ - Siblings* children(Simplex_handle sh) const { + Siblings const* children(Simplex_handle sh) const { GUDHI_CHECK(has_children(sh), std::invalid_argument("Simplex_tree::children - argument has no child")); return sh->second.children(); } @@ -1048,16 +1050,15 @@ class Simplex_tree { Vertex_handle vertex_one = *first; auto&& dict = sib->members(); auto insertion_result = dict.emplace(vertex_one, Node(sib, filt)); - std::pair result(insertion_result); - // update extra data structures in the insertion is successful - if (insertion_result.second) { - // Only required when insertion is successful - update_simplex_tree_after_node_insertion(insertion_result.first); - } + std::pair result(insertion_result); // const version of insertion_result for null_simplex() auto simplex_one = insertion_result.first; bool one_is_new = insertion_result.second; - if (!one_is_new) { + if (one_is_new) { + // update extra data structures in the insertion is successful + // Only required when insertion is successful + update_simplex_tree_after_node_insertion(insertion_result.first); + } else { if (filtration(simplex_one) > filt) { assign_filtration(simplex_one, filt); } else { @@ -1065,7 +1066,7 @@ class Simplex_tree { result.first = null_simplex(); } } - if (++first == last) return insertion_result; + if (++first == last) return 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)); @@ -1079,7 +1080,7 @@ class Simplex_tree { /** \brief Assign a value 'key' to the key of the simplex * represented by the Simplex_handle 'sh'. */ void assign_key(Simplex_handle sh, Simplex_key key) { - sh->second.assign_key(key); + _to_node_it(sh)->second.assign_key(key); } /** Returns the two Simplex_handle corresponding to the endpoints of @@ -1181,7 +1182,7 @@ class Simplex_tree { * Postfix actions : Finally, we add back the removed vertex into vertices, and remove this vertex from curr_nbVertices so that we didn't change the parameters. * If the vertices list is empty, we need to check if curr_nbVertices matches with the dimension of the cofaces asked. */ - void rec_coface(std::vector &vertices, Siblings *curr_sib, int curr_nbVertices, + void rec_coface(std::vector &vertices, Siblings const*curr_sib, int curr_nbVertices, std::vector& cofaces, bool star, int nbVertices) const { if (!(star || curr_nbVertices <= nbVertices)) // dimension of actual simplex <= nbVertices return; @@ -1565,7 +1566,7 @@ class Simplex_tree { { Node& node_u = static_cast(node_as_hook); //corresponding node, has label u Simplex_handle sh_u = simplex_handle_from_node(node_u); - Siblings const* sib_u = self_siblings(sh_u); + Siblings* sib_u = const_cast(self_siblings(sh_u)); if (sib_u->members().find(v) != sib_u->members().end()) { //v is the label of a sibling of node_u int curr_dim = dimension(sib_u); if (dim_max == -1 || curr_dim < dim_max){ @@ -1653,14 +1654,14 @@ class Simplex_tree { * k must be > 0 */ void create_local_expansion( - Simplex_handle sh_v //Node with label v which has just been inserted + Dictionary_it sh_v //Node with label v which has just been inserted , Siblings * curr_sib //Siblings containing the node sh_v , const Filtration_value& fil_uv //Fil value of the edge uv in the zz filtration , int k //Stopping condition for recursion based on max dim , std::vector &added_simplices) //range of all new simplices { //pick N^+(v) //intersect N^+(v) with labels y > v in curr_sib - Simplex_handle next_it = sh_v; + Dictionary_it next_it = sh_v; ++next_it; if (dimension_ > k) { @@ -1771,7 +1772,7 @@ class Simplex_tree { template static void intersection(std::vector >& intersection, Dictionary_it begin1, Dictionary_it end1, - Dictionary_it begin2, Dictionary_it end2, + Const_dictionary_it begin2, Const_dictionary_it end2, const Filtration_value& filtration_) { if (begin1 == end1 || begin2 == end2) return; // ----->> @@ -1958,7 +1959,7 @@ class Simplex_tree { private: template - void rec_for_each_simplex(Siblings* sib, int dim, Fun&& fun) { + void rec_for_each_simplex(const Siblings* sib, int dim, Fun&& fun) { Simplex_handle sh = sib->members().end(); GUDHI_CHECK(sh != sib->members().begin(), "Bug in Gudhi: only the root siblings may be empty"); do { @@ -2372,7 +2373,18 @@ class Simplex_tree { // unordered_map Vertex_handle v -> list of all Nodes with label v. std::unordered_map nodes_label_to_list_; - List_max_vertex* nodes_by_label(Vertex_handle v) const { + List_max_vertex* nodes_by_label(Vertex_handle v) { + if constexpr (Options::link_nodes_by_label) { + auto it_v = nodes_label_to_list_.find(v); + if (it_v != nodes_label_to_list_.end()) { + return &(it_v->second); + } else { + return nullptr; + } + } + return nullptr; + } + List_max_vertex const* nodes_by_label(Vertex_handle v) const { if constexpr (Options::link_nodes_by_label) { auto it_v = nodes_label_to_list_.find(v); if (it_v != nodes_label_to_list_.end()) { @@ -2386,7 +2398,7 @@ class Simplex_tree { /** \brief Helper method that returns the corresponding Simplex_handle from a member element defined by a node. */ - static Simplex_handle simplex_handle_from_node(Node& node) { + static Simplex_handle simplex_handle_from_node(const Node& node) { if constexpr (Options::stable_simplex_handles){ //Relies on the Dictionary type to be boost::container::map. //If the type changes or boost fundamentally changes something on the structure of their map, @@ -2398,8 +2410,8 @@ class Simplex_tree { // } //Requires an additional parameter "Vertex_handle label" which is the label of the node. - Dictionary_it testIt = node.children()->members().begin(); - Node* testNode = &testIt->second; + Const_dictionary_it testIt = node.children()->members().begin(); + const Node* testNode = &testIt->second; auto testIIt = testIt.get(); auto testPtr = testIIt.pointed_node(); //distance between node and pointer to map pair in memory @@ -2420,11 +2432,13 @@ class Simplex_tree { // false> decltype(testIIt) sh_ii; sh_ii = sh_ptr; //creates ``subiterator'' from pointer - Dictionary_it sh(sh_ii); //creates iterator from subiterator + Const_dictionary_it sh(sh_ii); //creates iterator from subiterator return sh; } else { - return (Simplex_handle)(boost::intrusive::get_parent_from_member(&node, &Dit_value_t::second)); + //node needs to be casted as non const, because a const pair* cannot be casted into a Simplex_handle + return (Simplex_handle)(boost::intrusive::get_parent_from_member(const_cast(&node), + &Const_dit_value_t::second)); } } @@ -2452,7 +2466,7 @@ class Simplex_tree { std::clog << "update_simplex_tree_before_node_removal" << std::endl; #endif // DEBUG_TRACES if constexpr (Options::link_nodes_by_label) { - sh->second.unlink_hooks(); // remove from lists of same label Nodes + _to_node_it(sh)->second.unlink_hooks(); // remove from lists of same label Nodes if (nodes_label_to_list_[sh->first].empty()) nodes_label_to_list_.erase(sh->first); } @@ -2544,7 +2558,7 @@ class Simplex_tree { private: /** \brief Serialize each element of the sibling and recursively call serialization. */ - char* rec_serialize(Siblings *sib, char* buffer) const { + char* rec_serialize(Siblings const *sib, char* buffer) const { char* ptr = buffer; ptr = Gudhi::simplex_tree::serialize_trivial(static_cast(sib->members().size()), ptr); #ifdef DEBUG_TRACES diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h index 1591ebd01b..76823ccd8d 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h @@ -206,7 +206,7 @@ class Simplex_tree_boundary_opposite_vertex_simplex_iterator : public boost::ite } // any end() iterator - explicit Simplex_tree_boundary_opposite_vertex_simplex_iterator(SimplexTree * st) + explicit Simplex_tree_boundary_opposite_vertex_simplex_iterator(SimplexTree const* st) : last_(st->null_vertex()), next_(st->null_vertex()), sib_(nullptr), @@ -215,7 +215,7 @@ class Simplex_tree_boundary_opposite_vertex_simplex_iterator : public boost::ite } template - Simplex_tree_boundary_opposite_vertex_simplex_iterator(SimplexTree * st, SimplexHandle sh) + Simplex_tree_boundary_opposite_vertex_simplex_iterator(SimplexTree const* st, SimplexHandle sh) : last_(sh->first), next_(st->null_vertex()), sib_(nullptr), @@ -258,8 +258,8 @@ class Simplex_tree_boundary_opposite_vertex_simplex_iterator : public boost::ite baov_.first = st_->null_simplex(); return; // ------>> } - Siblings * for_sib = sib_; - Siblings * new_sib = sib_->oncles(); + Siblings const* for_sib = sib_; + Siblings const* new_sib = sib_->oncles(); auto rit = suffix_.rbegin(); if constexpr (SimplexTree::Options::contiguous_vertices && !SimplexTree::Options::stable_simplex_handles) { if (new_sib == nullptr) { @@ -318,7 +318,7 @@ class Simplex_tree_complex_simplex_iterator : public boost::iterator_facade< st_(nullptr) { } - explicit Simplex_tree_complex_simplex_iterator(SimplexTree * st) + explicit Simplex_tree_complex_simplex_iterator(SimplexTree const* st) : sib_(nullptr), st_(st) { if (st == nullptr || st->root() == nullptr || st->root()->members().empty()) { @@ -394,7 +394,7 @@ class Simplex_tree_skeleton_simplex_iterator : public boost::iterator_facade< curr_dim_(0) { } - Simplex_tree_skeleton_simplex_iterator(SimplexTree * st, int dim_skel) + Simplex_tree_skeleton_simplex_iterator(SimplexTree const* st, int dim_skel) : sib_(nullptr), st_(st), dim_skel_(dim_skel), diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_star_simplex_iterators.h b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_star_simplex_iterators.h index 316ca31cb5..4b89adc004 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_star_simplex_iterators.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_star_simplex_iterators.h @@ -68,12 +68,12 @@ class Simplex_tree_optimized_cofaces_rooted_subtrees_simplex_iterator class is_coface { public: is_coface() : cpx_(nullptr) {} - is_coface(SimplexTree* cpx, Static_vertex_vector&& simp) : cpx_(cpx), simp_(simp) {} + is_coface(SimplexTree const* cpx, Static_vertex_vector&& simp) : cpx_(cpx), simp_(simp) {} // Return true iff traversing the Node upwards to the root reads a // coface of simp_ - bool operator()(typename SimplexTree::Hooks_simplex_base& curr_hooks) { - Node& curr_node = static_cast(curr_hooks); + bool operator()(const typename SimplexTree::Hooks_simplex_base& curr_hooks) { + const Node& curr_node = static_cast(curr_hooks); Simplex_handle sh = cpx_->simplex_handle_from_node(curr_node); // first Node must always have label simp_.begin(); we assume it is true auto&& rng = cpx_->simplex_vertex_range(sh); @@ -85,25 +85,25 @@ class Simplex_tree_optimized_cofaces_rooted_subtrees_simplex_iterator } private: - SimplexTree* cpx_; + SimplexTree const* cpx_; Static_vertex_vector simp_; // vertices of simplex, in reverse order }; - typedef boost::filter_iterator + typedef boost::filter_iterator Filtered_cofaces_simplex_iterator; // any end() iterator Simplex_tree_optimized_cofaces_rooted_subtrees_simplex_iterator() : predicate_(), st_(nullptr) {} - Simplex_tree_optimized_cofaces_rooted_subtrees_simplex_iterator(SimplexTree* cpx, + Simplex_tree_optimized_cofaces_rooted_subtrees_simplex_iterator(SimplexTree const* cpx, Static_vertex_vector&& simp) : predicate_(cpx, std::move(simp)), st_(cpx) { GUDHI_CHECK(!simp.empty(), std::invalid_argument("cannot call for cofaces of an empty simplex")); - auto list_ptr = st_->nodes_by_label(simp.front()); + const auto list_ptr = st_->nodes_by_label(simp.front()); GUDHI_CHECK(list_ptr != nullptr, std::runtime_error("invalid call to cofaces forest")); it_ = boost::make_filter_iterator(predicate_, list_ptr->begin(), list_ptr->end()); end_ = boost::make_filter_iterator(predicate_, list_ptr->end(), list_ptr->end()); - Node& curr_node = static_cast(*it_); + const Node& curr_node = static_cast(*it_); sh_ = st_->simplex_handle_from_node(curr_node); } @@ -128,7 +128,7 @@ class Simplex_tree_optimized_cofaces_rooted_subtrees_simplex_iterator st_ = nullptr; } //== end else { // update sh_ - Node& curr_node = static_cast(*it_); + const Node& curr_node = static_cast(*it_); sh_ = st_->simplex_handle_from_node(curr_node); } } @@ -136,7 +136,7 @@ class Simplex_tree_optimized_cofaces_rooted_subtrees_simplex_iterator // given a Node of label max_v, returns true if the associated simplex is a coface of the simplex {..., max_v}. The // predicate stores the vertices of the simplex whose star we compute. is_coface predicate_; - SimplexTree* st_; + SimplexTree const* st_; // filtered iterators over Nodes, filtered with predicate_ Filtered_cofaces_simplex_iterator it_; Filtered_cofaces_simplex_iterator end_; @@ -170,7 +170,7 @@ class Simplex_tree_optimized_star_simplex_iterator // any end() iterator Simplex_tree_optimized_star_simplex_iterator() : st_(nullptr) {} - Simplex_tree_optimized_star_simplex_iterator(SimplexTree* cpx, Static_vertex_vector&& simp) + Simplex_tree_optimized_star_simplex_iterator(SimplexTree const* cpx, Static_vertex_vector&& simp) : st_(cpx), it_(cpx, std::move(simp)), end_(), sh_(*it_), sib_(st_->self_siblings(sh_)), children_stack_() { if (it_ == end_) { st_ = nullptr; @@ -256,7 +256,7 @@ class Simplex_tree_optimized_star_simplex_iterator // Let s be the simplex in a complex C whose star is // iterated through. Let max_v denote the maximal label of vertices in s. - SimplexTree* st_; // Simplex tree for complex C + SimplexTree const* st_; // Simplex tree for complex C // The cofaces of s form a subforest of the simplex tree. The roots of trees in this // forest have label max_v. //[it_,end_) == range of Simplex_handles of the roots of the cofaces trees (any dim) @@ -267,7 +267,7 @@ class Simplex_tree_optimized_star_simplex_iterator // set of siblings containing sh_ in the Simplex_tree Siblings const* sib_; // // Save children in a list to avoid calling sib_->members().find(.) - std::vector children_stack_; + std::vector children_stack_; // true iff sh_ points to the root of a coface subtree bool is_root_; }; From 587c03d65cdbcf3999af0e560fcc4d7e51f38293 Mon Sep 17 00:00:00 2001 From: hschreiber Date: Mon, 12 Aug 2024 18:47:12 +0200 Subject: [PATCH 03/23] merge upstream + doc --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 122 ++++++++++-------- .../test/simplex_tree_unit_test.cpp | 5 +- 2 files changed, 73 insertions(+), 54 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 05c1cb393e..e14897087e 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -152,6 +152,11 @@ class Simplex_tree { typedef typename std::conditional::type Key_simplex_base; + /** + * @brief Equals @ref Filtration_value if @ref SimplexTreeOptions::store_filtration is false or the type + * corresponds to a native arithmetic type (int, double, etc.). Equals to a reference of @ref Filtration_value + * otherwise. + */ using Filtration_value_rep = std::conditional_t, Filtration_value&, Filtration_value>; @@ -159,20 +164,19 @@ class Simplex_tree { Filtration_simplex_base_real() : filt_(0) {} void assign_filtration(const Filtration_value& f) { filt_ = f; } const Filtration_value_rep filtration() const { return filt_; } - Filtration_value_rep filtration() { return filt_; } + Filtration_value& filtration_raw() { return filt_; } + const Filtration_value& filtration_raw() const { return filt_; } constexpr static Filtration_value_rep get_infinity(){ - if constexpr (!std::is_arithmetic_v) inf_ = _inf(); return inf_; } private: Filtration_value filt_; - static constexpr Filtration_value _inf() { - return std::numeric_limits::has_infinity ? std::numeric_limits::infinity() - : std::numeric_limits::max(); - } - static constexpr Filtration_value inf_ = _inf(); + + static constexpr Filtration_value inf_ = std::numeric_limits::has_infinity + ? std::numeric_limits::infinity() + : std::numeric_limits::max(); }; struct Filtration_simplex_base_dummy { Filtration_simplex_base_dummy() {} @@ -201,7 +205,7 @@ class Simplex_tree { typedef typename Const_dictionary_it::value_type Const_dit_value_t; struct return_first { - Vertex_handle operator()(const Dit_value_t& p_sh) const { + Vertex_handle operator()(const Const_dit_value_t& p_sh) const { return p_sh.first; } }; @@ -257,6 +261,12 @@ class Simplex_tree { typedef boost::transform_iterator Complex_vertex_iterator; /** \brief Range over the vertices of the simplicial complex. */ typedef boost::iterator_range Complex_vertex_range; + /** \brief Iterator over the vertices of the simplicial complex. + * + * 'value_type' is Vertex_handle. */ + typedef boost::transform_iterator Const_complex_vertex_iterator; + /** \brief Range over the vertices of the simplicial complex. */ + typedef boost::iterator_range Const_complex_vertex_range; /** \brief Iterator over the vertices of a simplex. * * 'value_type' is Vertex_handle. */ @@ -312,14 +322,12 @@ class Simplex_tree { /** \brief Returns a range over the vertices of the simplicial complex. * The order is increasing according to < on Vertex_handles.*/ - Complex_vertex_range complex_vertex_range() const { - return Complex_vertex_range( - boost::make_transform_iterator(root_.members_.begin(), return_first()), - boost::make_transform_iterator(root_.members_.end(), return_first())); + Const_complex_vertex_range complex_vertex_range() const { + return Const_complex_vertex_range(boost::make_transform_iterator(root_.members_.begin(), return_first()), + boost::make_transform_iterator(root_.members_.end(), return_first())); } Complex_vertex_range complex_vertex_range() { - return Complex_vertex_range( - boost::make_transform_iterator(root_.members_.begin(), return_first()), + return Complex_vertex_range(boost::make_transform_iterator(root_.members_.begin(), return_first()), boost::make_transform_iterator(root_.members_.end(), return_first())); } @@ -332,10 +340,6 @@ class Simplex_tree { return Complex_simplex_range(Complex_simplex_iterator(this), Complex_simplex_iterator()); } - Complex_simplex_range complex_simplex_range() { - return Complex_simplex_range(Complex_simplex_iterator(this), - Complex_simplex_iterator()); - } /** \brief Returns a range over the simplices of the dim-skeleton of the simplicial complex. * @@ -350,10 +354,6 @@ class Simplex_tree { return Skeleton_simplex_range(Skeleton_simplex_iterator(this, dim), Skeleton_simplex_iterator()); } - Skeleton_simplex_range skeleton_simplex_range(int dim) { - return Skeleton_simplex_range(Skeleton_simplex_iterator(this, dim), - Skeleton_simplex_iterator()); - } /** \brief Returns a range over the simplices of the simplicial complex, * in the order of the filtration. @@ -370,14 +370,8 @@ class Simplex_tree { * The filtration must be valid. If the filtration has not been initialized yet, the * method initializes it (i.e. order the simplices). If the complex has changed since the last time the filtration * was initialized, please call `clear_filtration()` or `initialize_filtration()` to recompute it. */ - Filtration_simplex_range const& filtration_simplex_range(Indexing_tag = Indexing_tag()) { - maybe_initialize_filtration(); - return filtration_vect_; - } Filtration_simplex_range const& filtration_simplex_range(Indexing_tag = Indexing_tag()) const { - GUDHI_CHECK(!filtration_vect_.empty() || Complex_simplex_range().empty(), - "Filtration range was not yet initialized. Please call `initialize_filtration`."); - if (filtration_vect_.empty() && !Complex_simplex_range().empty()) throw std::invalid_argument("filtration_simplex_range should be non const"); + maybe_initialize_filtration(); return filtration_vect_; } @@ -639,7 +633,7 @@ class Simplex_tree { return sh->second.filtration(); } - Dictionary_it _to_node_it(Simplex_handle sh){ + static Dictionary_it _to_node_it(Simplex_handle sh){ return const_cast(self_siblings(sh))->to_non_const_it(sh); } @@ -666,22 +660,7 @@ class Simplex_tree { * Called on the null_simplex, it returns infinity. * If SimplexTreeOptions::store_filtration is false, returns 0. */ - Filtration_value_rep filtration(Simplex_handle sh) { - if (sh == null_simplex()) { - // get_infinity rewrites the inf_ value in case someone changes the value of the reference - //the only way to avoid this would be to forbid the use of this method with null simplices: - - // if constexpr (Options::store_filtration && !std::is_arithmetic_v) - // throw std::invalid_argument( - // "Simplex handle does not redirect to a real simplex and a non constant reference cannot be returned. " - // "Please use the `const` version of the method."); - // else - return Filtration_simplex_base_real::get_infinity(); - } - - return sh->second.filtration(); - } - const Filtration_value_rep filtration(Simplex_handle sh) const { + static const Filtration_value_rep filtration(Simplex_handle sh) { if (sh != null_simplex()) { return sh->second.filtration(); } else { @@ -689,6 +668,42 @@ class Simplex_tree { } } + /** + * @brief Returns a reference to the filtration value of the given simplex. + * The given simplex handle has to be a valid handle and different from @ref null_simplex(). + * Furthermore, the option @ref SimplexTreeOptions::store_filtration has to be true. + * + * @param sh Valid simplex handle different from null_simplex() + * @return Reference to the filtration value. + */ + Filtration_value& filtration_raw(Simplex_handle sh) { + static_assert(Options::store_filtration, + "`filtration_raw` not available if SimplexTreeOptions::store_filtration is set to false."); + GUDHI_CHECK(sh != null_simplex(), "Simplex handle given to `filtration_raw` should not be a null simplex."); + + return _to_node_it(sh)->second.filtration_raw(); + } + /** + * @brief Const version of @ref filtration_raw(Simplex_handle sh). + */ + const Filtration_value& filtration_raw(Simplex_handle sh) const { + static_assert(Options::store_filtration, + "filtration_raw not available if SimplexTreeOptions::store_filtration is set to false."); + GUDHI_CHECK(sh != null_simplex(), "Simplex handle given to `filtration_raw` should not be a null simplex."); + + return sh->second.filtration_raw(); + } + + //because there is no direct access to Filtration_simplex_base_real::get_infinity() from the visible interface + /** + * @brief Returns true if and only if the filtration value associated to the given simplex is infinity. + * If the given handle is @ref null_simplex(), returns true. If @ref SimplexTreeOptions::store_filtration is set + * to false, returns always false. + * + * @param sh Simplex handle. + * @return true If the filtration value is equal to infinity or @p sh is @ref null_simplex(). + * @return false Otherwise. + */ static bool has_filtration_value_infinity(Simplex_handle sh){ if (sh == null_simplex()) return true; return sh->second.filtration() == Filtration_simplex_base_real::get_infinity(); @@ -720,7 +735,7 @@ class Simplex_tree { static Simplex_data& simplex_data(Simplex_handle sh) { GUDHI_CHECK(sh != null_simplex(), std::invalid_argument("Simplex_tree::simplex_data - no data associated to null_simplex")); - return sh->second.data(); + return _to_node_it(sh)->second.data(); } /** \brief Returns a Vertex_handle different from all Vertex_handles associated @@ -1136,7 +1151,7 @@ class Simplex_tree { * * Any insertion, deletion or change of filtration value invalidates this cache, * which can be cleared with clear_filtration(). */ - void initialize_filtration(bool ignore_infinite_values = false) { + void initialize_filtration(bool ignore_infinite_values = false) const { filtration_vect_.clear(); filtration_vect_.reserve(num_simplices()); for (Simplex_handle sh : complex_simplex_range()) { @@ -1164,7 +1179,7 @@ class Simplex_tree { /** \brief Initializes the filtration cache if it isn't initialized yet. * * Automatically called by filtration_simplex_range(). */ - void maybe_initialize_filtration() { + void maybe_initialize_filtration() const { if (filtration_vect_.empty()) { initialize_filtration(); } @@ -1325,7 +1340,7 @@ class Simplex_tree { * Reverse lexicographic order has the property to always consider the subsimplex of a simplex * to be smaller. The filtration function must be monotonic. */ struct is_before_in_filtration { - explicit is_before_in_filtration(Simplex_tree * st) + explicit is_before_in_filtration(Simplex_tree const* st) : st_(st) { } bool operator()(const Simplex_handle sh1, const Simplex_handle sh2) const { @@ -1337,7 +1352,7 @@ class Simplex_tree { return st_->reverse_lexicographic_order(sh1, sh2); } - Simplex_tree * st_; + Simplex_tree const* st_; }; public: @@ -2662,8 +2677,11 @@ class Simplex_tree { /** \brief Total number of simplices in the complex, without the empty simplex.*/ /** \brief Set of simplex tree Nodes representing the vertices.*/ Siblings root_; + + // all mutable as their content has no impact on the content of the simplex tree itself + // they correspond to some kind of cache or helper attributes. /** \brief Simplices ordered according to a filtration.*/ - std::vector filtration_vect_; + mutable std::vector filtration_vect_; /** \brief Upper bound on the dimension of the simplicial complex.*/ mutable int dimension_; mutable bool dimension_to_be_lowered_ = false; diff --git a/src/Simplex_tree/test/simplex_tree_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_unit_test.cpp index 75854b8f41..7a9d595358 100644 --- a/src/Simplex_tree/test/simplex_tree_unit_test.cpp +++ b/src/Simplex_tree/test/simplex_tree_unit_test.cpp @@ -63,13 +63,14 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_tree_filtrations, typeST, list_of_tested_v BOOST_CHECK(st.filtration(res1) == 0); if constexpr (typeST::Options::store_filtration && !std::is_arithmetic_v){ - typename typeST::Filtration_value& fil = st.filtration(res1); + typename typeST::Filtration_value& fil = st.filtration_raw(res1); fil = 5; BOOST_CHECK(st.filtration(res1) == 5); } - BOOST_CHECK(st.filtration(st.find({0,1})) == 2); + auto fil2 = st.filtration_raw(st.find({0,1})); + BOOST_CHECK(fil2 == 2); BOOST_CHECK(st.has_filtration_value_infinity(st.null_simplex())); } From b7f4649d2d8bc2368d78113ed88d6786f444b9a7 Mon Sep 17 00:00:00 2001 From: hschreiber Date: Tue, 15 Oct 2024 16:53:22 +0200 Subject: [PATCH 04/23] upstream merge --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 5496dcff9e..9265fd94a9 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -1634,10 +1634,10 @@ class Simplex_tree { * 2- All Node in the members of sib, with label @p x and @p x < @p v, * need in turn a local_expansion by @p v iff N^+(x) contains @p v. */ - void compute_punctual_expansion( Vertex_handle v - , Siblings * sib - , Filtration_value fil - , int k //k == dim_max - dimension simplices in sib + void compute_punctual_expansion( Vertex_handle v + , Siblings * sib + , Filtration_value fil + , int k //k == dim_max - dimension simplices in sib , std::vector& added_simplices ) { //insertion always succeeds because the edge {u,v} used to not be here. auto res_ins_v = sib->members().emplace(v, Node(sib,fil)); @@ -1683,10 +1683,10 @@ class Simplex_tree { * k must be > 0 */ void create_local_expansion( - Dictionary_it sh_v //Node with label v which has just been inserted - , Siblings * curr_sib //Siblings containing the node sh_v - , Filtration_value fil_uv //Fil value of the edge uv in the zz filtration - , int k //Stopping condition for recursion based on max dim + Dictionary_it sh_v //Node with label v which has just been inserted + , Siblings * curr_sib //Siblings containing the node sh_v + , Filtration_value fil_uv //Fil value of the edge uv in the zz filtration + , int k //Stopping condition for recursion based on max dim , std::vector &added_simplices) //range of all new simplices { //pick N^+(v) //intersect N^+(v) with labels y > v in curr_sib @@ -1710,9 +1710,9 @@ class Simplex_tree { * Only called in the case of `void insert_edge_as_flag(...)`. */ void siblings_expansion( - Siblings * siblings // must contain elements - , Filtration_value fil - , int k // == max_dim expansion - dimension curr siblings + Siblings * siblings // must contain elements + , Filtration_value fil + , int k // == max_dim expansion - dimension curr siblings , std::vector & added_simplices ) { if (dimension_ > k) { @@ -2054,7 +2054,7 @@ class Simplex_tree { * than it was before. However, `upper_bound_dimension()` will return the old value, which remains a valid upper * bound. If you care, you can call `dimension()` to recompute the exact dimension. */ - bool prune_above_filtration(const Filtration_value& filtration) { + bool prune_above_filtration(Filtration_value filtration) { if (std::numeric_limits::has_infinity && filtration == std::numeric_limits::infinity()) return false; // ---->> bool modified = rec_prune_above_filtration(root(), filtration); @@ -2064,7 +2064,7 @@ class Simplex_tree { } private: - bool rec_prune_above_filtration(Siblings* sib, const Filtration_value& filt) { + bool rec_prune_above_filtration(Siblings* sib, Filtration_value filt) { auto&& list = sib->members(); bool modified = false; bool emptied = false; From e42504940658cdabc7d68e1b816545241013935f Mon Sep 17 00:00:00 2001 From: hschreiber Date: Tue, 15 Oct 2024 17:12:02 +0200 Subject: [PATCH 05/23] restore Persistent_cohomology.h --- .../include/gudhi/Persistent_cohomology.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Persistent_cohomology/include/gudhi/Persistent_cohomology.h b/src/Persistent_cohomology/include/gudhi/Persistent_cohomology.h index e629ce7933..e0f959c2c6 100644 --- a/src/Persistent_cohomology/include/gudhi/Persistent_cohomology.h +++ b/src/Persistent_cohomology/include/gudhi/Persistent_cohomology.h @@ -562,7 +562,7 @@ class Persistent_cohomology { void output_diagram(std::ostream& ostream = std::cout) { cmp_intervals_by_length cmp(cpx_); std::sort(std::begin(persistent_pairs_), std::end(persistent_pairs_), cmp); - for (const auto& pair : persistent_pairs_) { + for (auto pair : persistent_pairs_) { ostream << get<2>(pair) << " " << cpx_->dimension(get<0>(pair)) << " " << cpx_->filtration(get<0>(pair)) << " " << cpx_->filtration(get<1>(pair)) << " " << std::endl; @@ -574,7 +574,7 @@ class Persistent_cohomology { diagram_out.exceptions(diagram_out.failbit); cmp_intervals_by_length cmp(cpx_); std::sort(std::begin(persistent_pairs_), std::end(persistent_pairs_), cmp); - for (const auto& pair : persistent_pairs_) { + for (auto pair : persistent_pairs_) { diagram_out << cpx_->dimension(get<0>(pair)) << " " << cpx_->filtration(get<0>(pair)) << " " << cpx_->filtration(get<1>(pair)) << std::endl; @@ -589,7 +589,7 @@ class Persistent_cohomology { // size for an empty complex std::vector betti_numbers(std::max(dim_max_, 0)); - for (const auto& pair : persistent_pairs_) { + for (auto pair : persistent_pairs_) { // Count never ended persistence intervals if (cpx_->null_simplex() == get<1>(pair)) { // Increment corresponding betti number @@ -607,7 +607,7 @@ class Persistent_cohomology { int betti_number(int dimension) const { int betti_number = 0; - for (const auto& pair : persistent_pairs_) { + for (auto pair : persistent_pairs_) { // Count never ended persistence intervals if (cpx_->null_simplex() == get<1>(pair)) { if (cpx_->dimension(get<0>(pair)) == dimension) { @@ -628,7 +628,7 @@ class Persistent_cohomology { // Init Betti numbers vector with zeros until Simplicial complex dimension and don't allocate a vector of negative // size for an empty complex std::vector betti_numbers(std::max(dim_max_, 0)); - for (const auto& pair : persistent_pairs_) { + for (auto pair : persistent_pairs_) { // Count persistence intervals that covers the given interval // null_simplex test : if the function is called with to=+infinity, we still get something useful. And it will // still work if we change the complex filtration function to reject null simplices. @@ -650,7 +650,7 @@ class Persistent_cohomology { int persistent_betti_number(int dimension, Filtration_value from, Filtration_value to) const { int betti_number = 0; - for (const auto& pair : persistent_pairs_) { + for (auto pair : persistent_pairs_) { // Count persistence intervals that covers the given interval // null_simplex test : if the function is called with to=+infinity, we still get something useful. And it will // still work if we change the complex filtration function to reject null simplices. From 8067ba7efb2d5b9d4b6abadf12a280aa449bd96a Mon Sep 17 00:00:00 2001 From: hschreiber Date: Tue, 15 Oct 2024 17:54:42 +0200 Subject: [PATCH 06/23] removal of non const ranges --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 78 +++++++++---------- .../Simplex_tree/Simplex_tree_iterators.h | 1 - .../Simplex_tree/Simplex_tree_siblings.h | 3 - 3 files changed, 39 insertions(+), 43 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 9265fd94a9..699c6075d0 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -189,10 +189,9 @@ class Simplex_tree { typedef typename Dictionary::iterator Dictionary_it; typedef typename Dictionary::const_iterator Const_dictionary_it; typedef typename Dictionary_it::value_type Dit_value_t; - typedef typename Const_dictionary_it::value_type Const_dit_value_t; struct return_first { - Vertex_handle operator()(const Const_dit_value_t& p_sh) const { + Vertex_handle operator()(const Dit_value_t& p_sh) const { return p_sh.first; } }; @@ -209,11 +208,11 @@ class Simplex_tree { using Optimized_star_simplex_range = boost::iterator_range; class Fast_cofaces_predicate { - Simplex_tree* st_; + Simplex_tree const* st_; int codim_; int dim_; public: - Fast_cofaces_predicate(Simplex_tree* st, int codim, int dim) + Fast_cofaces_predicate(Simplex_tree const* st, int codim, int dim) : st_(st), codim_(codim), dim_(codim + dim) {} bool operator()( const Simplex_handle iter ) const { if (codim_ == 0) @@ -245,15 +244,15 @@ class Simplex_tree { /** \brief Iterator over the vertices of the simplicial complex. * * 'value_type' is Vertex_handle. */ - typedef boost::transform_iterator Complex_vertex_iterator; + typedef boost::transform_iterator Complex_vertex_iterator; /** \brief Range over the vertices of the simplicial complex. */ typedef boost::iterator_range Complex_vertex_range; - /** \brief Iterator over the vertices of the simplicial complex. - * - * 'value_type' is Vertex_handle. */ - typedef boost::transform_iterator Const_complex_vertex_iterator; - /** \brief Range over the vertices of the simplicial complex. */ - typedef boost::iterator_range Const_complex_vertex_range; + // /** \brief Iterator over the vertices of the simplicial complex. + // * + // * 'value_type' is Vertex_handle. */ + // typedef boost::transform_iterator Const_complex_vertex_iterator; + // /** \brief Range over the vertices of the simplicial complex. */ + // typedef boost::iterator_range Const_complex_vertex_range; /** \brief Iterator over the vertices of a simplex. * * 'value_type' is Vertex_handle. */ @@ -309,11 +308,11 @@ class Simplex_tree { /** \brief Returns a range over the vertices of the simplicial complex. * The order is increasing according to < on Vertex_handles.*/ - Const_complex_vertex_range complex_vertex_range() const { - return Const_complex_vertex_range(boost::make_transform_iterator(root_.members_.begin(), return_first()), - boost::make_transform_iterator(root_.members_.end(), return_first())); - } - Complex_vertex_range complex_vertex_range() { + // Const_complex_vertex_range complex_vertex_range() const { + // return Const_complex_vertex_range(boost::make_transform_iterator(root_.members_.begin(), return_first()), + // boost::make_transform_iterator(root_.members_.end(), return_first())); + // } + Complex_vertex_range complex_vertex_range() const { return Complex_vertex_range(boost::make_transform_iterator(root_.members_.begin(), return_first()), boost::make_transform_iterator(root_.members_.end(), return_first())); } @@ -388,11 +387,11 @@ class Simplex_tree { * of the simplex. * * @param[in] sh Simplex for which the boundary is computed. */ - template - Boundary_simplex_range boundary_simplex_range(SimplexHandle sh) { - return Boundary_simplex_range(Boundary_simplex_iterator(this, sh), - Boundary_simplex_iterator(this)); - } + // template + // Boundary_simplex_range boundary_simplex_range(SimplexHandle sh) { + // return Boundary_simplex_range(Boundary_simplex_iterator(this, sh), + // Boundary_simplex_iterator(this)); + // } template Boundary_simplex_range boundary_simplex_range(SimplexHandle sh) const { return Boundary_simplex_range(Boundary_simplex_iterator(this, sh), @@ -410,11 +409,11 @@ class Simplex_tree { * * @param[in] sh Simplex for which the boundary is computed. */ - template - Boundary_opposite_vertex_simplex_range boundary_opposite_vertex_simplex_range(SimplexHandle sh) { - return Boundary_opposite_vertex_simplex_range(Boundary_opposite_vertex_simplex_iterator(this, sh), - Boundary_opposite_vertex_simplex_iterator(this)); - } + // template + // Boundary_opposite_vertex_simplex_range boundary_opposite_vertex_simplex_range(SimplexHandle sh) { + // return Boundary_opposite_vertex_simplex_range(Boundary_opposite_vertex_simplex_iterator(this, sh), + // Boundary_opposite_vertex_simplex_iterator(this)); + // } template Boundary_opposite_vertex_simplex_range boundary_opposite_vertex_simplex_range(SimplexHandle sh) const { return Boundary_opposite_vertex_simplex_range(Boundary_opposite_vertex_simplex_iterator(this, sh), @@ -798,7 +797,7 @@ class Simplex_tree { public: /** \brief Returns the number of simplices of each dimension in the simplex tree. */ - std::vector num_simplices_by_dimension() { + std::vector num_simplices_by_dimension() const { if (is_empty()) return {}; // std::min in case the upper bound got crazy std::vector res(std::min(upper_bound_dimension()+1, max_dimension()+1)); @@ -1130,6 +1129,7 @@ class Simplex_tree { } public: + /** Returns a pointer to the root nodes of the simplex tree. */ Siblings* root() { return &root_; } /** Returns a pointer to the root nodes of the simplex tree. */ @@ -1260,9 +1260,9 @@ class Simplex_tree { * Simplex_tree::Simplex_handle range for an optimized search for the star of a simplex when * SimplexTreeOptions::link_nodes_by_label is true. */ - Cofaces_simplex_range star_simplex_range(const Simplex_handle simplex) { - return cofaces_simplex_range(simplex, 0); - } + // Cofaces_simplex_range star_simplex_range(const Simplex_handle simplex) { + // return cofaces_simplex_range(simplex, 0); + // } Cofaces_simplex_range star_simplex_range(const Simplex_handle simplex) const { return cofaces_simplex_range(simplex, 0); } @@ -1277,16 +1277,16 @@ class Simplex_tree { * Simplex_tree::Simplex_handle range for an optimized search for the coface of a simplex when * SimplexTreeOptions::link_nodes_by_label is true. */ - Cofaces_simplex_range cofaces_simplex_range(const Simplex_handle simplex, int codimension) { - return _cofaces_simplex_range(simplex, codimension); - } + // Cofaces_simplex_range cofaces_simplex_range(const Simplex_handle simplex, int codimension) { + // return _cofaces_simplex_range(simplex, codimension); + // } Cofaces_simplex_range cofaces_simplex_range(const Simplex_handle simplex, int codimension) const{ return _cofaces_simplex_range(simplex, codimension); } private: - Cofaces_simplex_range _cofaces_simplex_range(const Simplex_handle simplex, int codimension) { + Cofaces_simplex_range _cofaces_simplex_range(const Simplex_handle simplex, int codimension) const { // codimension must be positive or null integer assert(codimension >= 0); @@ -1972,7 +1972,7 @@ class Simplex_tree { * the children of this simplex (a subset of the cofaces). */ template - void for_each_simplex(Fun&& fun) { + void for_each_simplex(Fun&& fun) const { // Wrap callback so it always returns bool auto f = [&fun](Simplex_handle sh, int dim) -> bool { if constexpr (std::is_same_v) { @@ -1988,7 +1988,7 @@ class Simplex_tree { private: template - void rec_for_each_simplex(const Siblings* sib, int dim, Fun&& fun) { + void rec_for_each_simplex(const Siblings* sib, int dim, Fun&& fun) const { Simplex_handle sh = sib->members().end(); GUDHI_CHECK(sh != sib->members().begin(), "Bug in Gudhi: only the root siblings may be empty"); do { @@ -2466,8 +2466,8 @@ class Simplex_tree { return sh; } else { //node needs to be casted as non const, because a const pair* cannot be casted into a Simplex_handle - return (Simplex_handle)(boost::intrusive::get_parent_from_member(const_cast(&node), - &Const_dit_value_t::second)); + return (Simplex_handle)(boost::intrusive::get_parent_from_member(const_cast(&node), + &Dit_value_t::second)); } } @@ -2694,7 +2694,7 @@ class Simplex_tree { // Print a Simplex_tree in os. template -std::ostream& operator<<(std::ostream & os, Simplex_tree & st) { +std::ostream& operator<<(std::ostream & os, const Simplex_tree & st) { for (auto sh : st.filtration_simplex_range()) { os << st.dimension(sh) << " "; for (auto v : st.simplex_vertex_range(sh)) { diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h index 76823ccd8d..72af23a3dc 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_iterators.h @@ -16,7 +16,6 @@ #include -#include #include // for std::pair namespace Gudhi { diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_siblings.h b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_siblings.h index c259eb72bc..79e75a42d3 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_siblings.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_siblings.h @@ -15,9 +15,6 @@ #include -#include -#include - namespace Gudhi { /** \addtogroup simplex_tree From 06834b922f3535277c050278780eb25d8e8eba93 Mon Sep 17 00:00:00 2001 From: hschreiber Date: Tue, 15 Oct 2024 17:57:21 +0200 Subject: [PATCH 07/23] removal of non const ranges --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 699c6075d0..56a1faa4f8 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -247,12 +247,6 @@ class Simplex_tree { typedef boost::transform_iterator Complex_vertex_iterator; /** \brief Range over the vertices of the simplicial complex. */ typedef boost::iterator_range Complex_vertex_range; - // /** \brief Iterator over the vertices of the simplicial complex. - // * - // * 'value_type' is Vertex_handle. */ - // typedef boost::transform_iterator Const_complex_vertex_iterator; - // /** \brief Range over the vertices of the simplicial complex. */ - // typedef boost::iterator_range Const_complex_vertex_range; /** \brief Iterator over the vertices of a simplex. * * 'value_type' is Vertex_handle. */ @@ -308,10 +302,6 @@ class Simplex_tree { /** \brief Returns a range over the vertices of the simplicial complex. * The order is increasing according to < on Vertex_handles.*/ - // Const_complex_vertex_range complex_vertex_range() const { - // return Const_complex_vertex_range(boost::make_transform_iterator(root_.members_.begin(), return_first()), - // boost::make_transform_iterator(root_.members_.end(), return_first())); - // } Complex_vertex_range complex_vertex_range() const { return Complex_vertex_range(boost::make_transform_iterator(root_.members_.begin(), return_first()), boost::make_transform_iterator(root_.members_.end(), return_first())); @@ -387,11 +377,6 @@ class Simplex_tree { * of the simplex. * * @param[in] sh Simplex for which the boundary is computed. */ - // template - // Boundary_simplex_range boundary_simplex_range(SimplexHandle sh) { - // return Boundary_simplex_range(Boundary_simplex_iterator(this, sh), - // Boundary_simplex_iterator(this)); - // } template Boundary_simplex_range boundary_simplex_range(SimplexHandle sh) const { return Boundary_simplex_range(Boundary_simplex_iterator(this, sh), @@ -409,11 +394,6 @@ class Simplex_tree { * * @param[in] sh Simplex for which the boundary is computed. */ - // template - // Boundary_opposite_vertex_simplex_range boundary_opposite_vertex_simplex_range(SimplexHandle sh) { - // return Boundary_opposite_vertex_simplex_range(Boundary_opposite_vertex_simplex_iterator(this, sh), - // Boundary_opposite_vertex_simplex_iterator(this)); - // } template Boundary_opposite_vertex_simplex_range boundary_opposite_vertex_simplex_range(SimplexHandle sh) const { return Boundary_opposite_vertex_simplex_range(Boundary_opposite_vertex_simplex_iterator(this, sh), @@ -1260,9 +1240,6 @@ class Simplex_tree { * Simplex_tree::Simplex_handle range for an optimized search for the star of a simplex when * SimplexTreeOptions::link_nodes_by_label is true. */ - // Cofaces_simplex_range star_simplex_range(const Simplex_handle simplex) { - // return cofaces_simplex_range(simplex, 0); - // } Cofaces_simplex_range star_simplex_range(const Simplex_handle simplex) const { return cofaces_simplex_range(simplex, 0); } @@ -1277,10 +1254,6 @@ class Simplex_tree { * Simplex_tree::Simplex_handle range for an optimized search for the coface of a simplex when * SimplexTreeOptions::link_nodes_by_label is true. */ - // Cofaces_simplex_range cofaces_simplex_range(const Simplex_handle simplex, int codimension) { - // return _cofaces_simplex_range(simplex, codimension); - // } - Cofaces_simplex_range cofaces_simplex_range(const Simplex_handle simplex, int codimension) const{ return _cofaces_simplex_range(simplex, codimension); } From 805516049f36e45b69b520fa7ef5df12b1625357 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Fri, 18 Oct 2024 11:34:23 +0200 Subject: [PATCH 08/23] Fix operator== and != for constness. Add some TU for constness --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 80 +++++++++---------- src/Simplex_tree/test/CMakeLists.txt | 3 + .../simplex_tree_ctor_and_move_unit_test.cpp | 6 +- .../simplex_tree_edge_expansion_unit_test.cpp | 5 -- .../test/simplex_tree_unit_test.cpp | 20 ++--- 5 files changed, 56 insertions(+), 58 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 56a1faa4f8..3d35884f95 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -61,7 +61,7 @@ namespace Gudhi { -/** \addtogroup simplex_tree +/** \addtogroup simplex_tree * @{ */ @@ -281,12 +281,12 @@ class Simplex_tree { typedef Simplex_tree_complex_simplex_iterator Complex_simplex_iterator; /** \brief Range over the simplices of the simplicial complex. */ typedef boost::iterator_range Complex_simplex_range; - /** \brief Iterator over the simplices of the skeleton of the simplicial complex, for a given + /** \brief Iterator over the simplices of the skeleton of the simplicial complex, for a given * dimension. * * 'value_type' is Simplex_handle. */ typedef Simplex_tree_skeleton_simplex_iterator Skeleton_simplex_iterator; - /** \brief Range over the simplices of the skeleton of the simplicial complex, for a given + /** \brief Range over the simplices of the skeleton of the simplicial complex, for a given * dimension. */ typedef boost::iterator_range Skeleton_simplex_range; /** \brief Range over the simplices of the simplicial complex, ordered by the filtration. */ @@ -300,7 +300,7 @@ class Simplex_tree { /** \name Range and iterator methods * @{ */ - /** \brief Returns a range over the vertices of the simplicial complex. + /** \brief Returns a range over the vertices of the simplicial complex. * The order is increasing according to < on Vertex_handles.*/ Complex_vertex_range complex_vertex_range() const { return Complex_vertex_range(boost::make_transform_iterator(root_.members_.begin(), return_first()), @@ -418,7 +418,7 @@ class Simplex_tree { * are already implicitly convertible if the concept of @ref SimplexTreeOptions is respected (note that there is * an eventual loss of precision or an undefined behaviour if a value is converted into a new type too small to * contain it). Any extra data (@ref Simplex_data) stored in the simplices are ignored in the copy for now. - * + * * @tparam OtherSimplexTreeOptions Options of the given simplex tree. * @tparam F Method taking an OtherSimplexTreeOptions::Filtration_value as input and returning an * Options::Filtration_value. @@ -614,7 +614,7 @@ class Simplex_tree { /** \brief Checks if two simplex trees are equal. */ template - bool operator==(Simplex_tree& st2) const { + bool operator==(const Simplex_tree& st2) const { if ((null_vertex_ != st2.null_vertex_) || (dimension_ != st2.dimension_ && !dimension_to_be_lowered_ && !st2.dimension_to_be_lowered_)) return false; @@ -623,7 +623,7 @@ class Simplex_tree { /** \brief Checks if two simplex trees are different. */ template - bool operator!=(Simplex_tree& st2) const { + bool operator!=(const Simplex_tree& st2) const { return (!(*this == st2)); } @@ -762,7 +762,7 @@ class Simplex_tree { /** * @brief Returns the dimension of the given sibling simplices. - * + * * @param curr_sib Pointer to the sibling container. * @return Height of the siblings in the tree (root counts as zero to make the height correspond to the dimension). */ @@ -922,7 +922,7 @@ class Simplex_tree { * we assign this simplex with the new value 'filtration', and set the Simplex_handle field of the * output pair to the Simplex_handle of the simplex. Otherwise, we set the Simplex_handle part to * null_simplex. - * + * */ template > std::pair insert_simplex_raw(const RandomVertexHandleRange& simplex, @@ -1236,7 +1236,7 @@ class Simplex_tree { * \param simplex represent the simplex of which we search the star * \return Vector of Simplex_tree::Simplex_handle (empty vector if no star found) when * SimplexTreeOptions::link_nodes_by_label is false. - * + * * Simplex_tree::Simplex_handle range for an optimized search for the star of a simplex when * SimplexTreeOptions::link_nodes_by_label is true. */ @@ -1250,7 +1250,7 @@ class Simplex_tree { * cofaces (equivalent of star function) * \return Vector of Simplex_tree::Simplex_handle (empty vector if no cofaces found) when * SimplexTreeOptions::link_nodes_by_label is false. - * + * * Simplex_tree::Simplex_handle range for an optimized search for the coface of a simplex when * SimplexTreeOptions::link_nodes_by_label is true. */ @@ -1463,7 +1463,7 @@ class Simplex_tree { * @brief Adds a new vertex or a new edge in a flag complex, as well as all * simplices of its star, defined to maintain the property * of the complex to be a flag complex, truncated at dimension dim_max. - * To insert a new edge, the two given vertex handles have to correspond + * To insert a new edge, the two given vertex handles have to correspond * to the two end points of the edge. To insert a new vertex, the handles * have to be twice the same and correspond to the number you want assigned * to it. I.e., to insert vertex \f$i\f$, give \f$u = v = i\f$. @@ -1471,7 +1471,7 @@ class Simplex_tree { * the simplex tree, so the behaviour is undefined if called on an existing * edge. Also, the vertices of an edge have to be inserted before the edge. * - * @param[in] u,v Vertex_handle representing the new edge + * @param[in] u,v Vertex_handle representing the new edge * (@p v != @p u) or the new vertex (@p v == @p u). * @param[in] fil Filtration value of the edge. * @param[in] dim_max Maximal dimension of the expansion. @@ -1489,8 +1489,8 @@ class Simplex_tree { * filtration values, the method `make_filtration_non_decreasing()` has to be * called at the end of the insertions to restore the intended filtration. * Note that even then, an edge has to be inserted after its vertices. - * @warning The method assumes that the given edge or vertex was not already - * contained in the simplex tree, so the behaviour is undefined if called on + * @warning The method assumes that the given edge or vertex was not already + * contained in the simplex tree, so the behaviour is undefined if called on * an existing simplex. */ void insert_edge_as_flag( Vertex_handle u @@ -2095,7 +2095,7 @@ class Simplex_tree { bool prune_above_dimension(int dimension) { if (dimension >= dimension_) return false; - + bool modified = false; if (dimension < 0) { if (num_vertices() > 0) { @@ -2197,17 +2197,17 @@ class Simplex_tree { } } - /** \brief Retrieve the original filtration value for a given simplex in the Simplex_tree. Since the + /** \brief Retrieve the original filtration value for a given simplex in the Simplex_tree. Since the * computation of extended persistence requires modifying the filtration values, this function can be used * to recover the original values. Moreover, computing extended persistence requires adding new simplices * in the Simplex_tree. Hence, this function also outputs the type of each simplex. It can be either UP (which means * that the simplex was present originally, and is thus part of the ascending extended filtration), DOWN (which means * that the simplex is the cone of an original simplex, and is thus part of the descending extended filtration) or * EXTRA (which means the simplex is the cone point). See the definition of Extended_simplex_type. Note that if the simplex type is DOWN, the original filtration value - * is set to be the original filtration value of the corresponding (not coned) original simplex. + * is set to be the original filtration value of the corresponding (not coned) original simplex. * \pre This function should be called only if `extend_filtration()` has been called first! * \post The output filtration value is supposed to be the same, but might be a little different, than the - * original filtration value, due to the internal transformation (scaling to [-2,-1]) that is + * original filtration value, due to the internal transformation (scaling to [-2,-1]) that is * performed on the original filtration values during the computation of extended persistence. * @param[in] f Filtration value of the simplex in the extended (i.e., modified) filtration. * @param[in] efd Structure containing the minimum and maximum values of the original filtration. This the output of `extend_filtration()`. @@ -2227,12 +2227,12 @@ class Simplex_tree { return p; }; - /** \brief Extend filtration for computing extended persistence. - * This function only uses the filtration values at the 0-dimensional simplices, - * and computes the extended persistence diagram induced by the lower-star filtration - * computed with these values. - * \post Note that after calling this function, the filtration - * values are actually modified. The function `decode_extended_filtration()` + /** \brief Extend filtration for computing extended persistence. + * This function only uses the filtration values at the 0-dimensional simplices, + * and computes the extended persistence diagram induced by the lower-star filtration + * computed with these values. + * \post Note that after calling this function, the filtration + * values are actually modified. The function `decode_extended_filtration()` * retrieves the original values and outputs the extended simplex type. * @exception std::invalid_argument In debug mode if the Simplex tree contains a vertex with the largest * Vertex_handle, as this method requires to create an extra vertex internally. @@ -2253,7 +2253,7 @@ class Simplex_tree { maxval = std::max(maxval, f); maxvert = std::max(sh->first, maxvert); } - + GUDHI_CHECK(maxvert < std::numeric_limits::max(), std::invalid_argument("Simplex_tree contains a vertex with the largest Vertex_handle")); maxvert++; @@ -2292,7 +2292,7 @@ class Simplex_tree { // Automatically assign good values for simplices this->make_filtration_non_decreasing(); - // Return the filtration data + // Return the filtration data return Extended_filtration_data(minval, maxval); } @@ -2421,14 +2421,14 @@ class Simplex_tree { //decltype(testPtr) = boost::intrusive::compact_rbtree_node* decltype(testPtr) sh_ptr = decltype(testPtr)((const char*)(&node) - shift); //shifts from node to pointer - //decltype(testIIt) = + //decltype(testIIt) = //boost::intrusive::tree_iterator< // boost::intrusive::bhtraits< // boost::container::base_node< // std::pair>, // boost::container::dtl::intrusive_tree_hook, true>, - // boost::intrusive::rbtree_node_traits, - // boost::intrusive::normal_link, + // boost::intrusive::rbtree_node_traits, + // boost::intrusive::normal_link, // boost::intrusive::dft_tag, // 3>, // false> @@ -2507,9 +2507,9 @@ class Simplex_tree { public: /** @private @brief Returns the serialization required buffer size. - * + * * @return The exact serialization required size in number of bytes. - * + * * @warning It is meant to return the same size with the same SimplexTreeOptions and on a computer with the same * architecture. */ @@ -2522,14 +2522,14 @@ class Simplex_tree { #endif // DEBUG_TRACES return buffer_byte_size; } - + /** @private @brief Serialize the Simplex tree - Flatten it in a user given array of char - * + * * @param[in] buffer An array of char allocated with enough space (cf. Gudhi::simplex_tree::get_serialization_size) * @param[in] buffer_size The buffer size. - * + * * @exception std::invalid_argument If serialization does not match exactly the buffer_size value. - * + * * @warning Serialize/Deserialize is not portable. It is meant to be read in a Simplex_tree with the same * SimplexTreeOptions and on a computer with the same architecture. */ @@ -2590,16 +2590,16 @@ class Simplex_tree { public: /** @private @brief Deserialize the array of char (flatten version of the tree) to initialize a Simplex tree. * It is the user's responsibility to provide an 'empty' Simplex_tree, there is no guarantee otherwise. - * + * * @param[in] buffer A pointer on a buffer that contains a serialized Simplex_tree. * @param[in] buffer_size The size of the buffer. - * + * * @exception std::invalid_argument In case the deserialization does not finish at the correct buffer_size. * @exception std::logic_error In debug mode, if the Simplex_tree is not 'empty'. - * + * * @warning Serialize/Deserialize is not portable. It is meant to be read in a Simplex_tree with the same * SimplexTreeOptions and on a computer with the same architecture. - * + * */ void deserialize(const char* buffer, const std::size_t buffer_size) { GUDHI_CHECK(num_vertices() == 0, std::logic_error("Simplex_tree::deserialize - Simplex_tree must be empty")); diff --git a/src/Simplex_tree/test/CMakeLists.txt b/src/Simplex_tree/test/CMakeLists.txt index 5f441b338b..5fd5366ba1 100644 --- a/src/Simplex_tree/test/CMakeLists.txt +++ b/src/Simplex_tree/test/CMakeLists.txt @@ -29,3 +29,6 @@ gudhi_add_boost_test(Simplex_tree_edge_expansion_unit_test) add_executable_with_targets(Simplex_tree_extended_filtration_test_unit simplex_tree_extended_filtration_unit_test.cpp TBB::tbb) gudhi_add_boost_test(Simplex_tree_extended_filtration_test_unit) + +add_executable_with_targets(Simplex_tree_constness_test_unit simplex_tree_constness_unit_test.cpp TBB::tbb) +gudhi_add_boost_test(Simplex_tree_constness_test_unit) 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 1d370adfef..a5a7f5e1e6 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 @@ -42,7 +42,7 @@ std::string print_filtration_value(std::vector fil){ } template -void print_simplex_filtration(Simplex_tree& st, const std::string& msg) { +void print_simplex_filtration(const Simplex_tree& st, const std::string& msg) { // Required before browsing through filtration values st.initialize_filtration(); @@ -337,7 +337,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_tree_custom_copy_constructor_key, typeST, } template -std::vector> get_star(Simplex_tree& st) { +std::vector> get_star(const Simplex_tree& st) { std::vector> output; auto sh = st.find({0,1}); if (sh != st.null_simplex()) @@ -358,7 +358,7 @@ std::vector> get_star(Simplex_ } BOOST_AUTO_TEST_CASE(simplex_fast_cofaces_rule_of_five) { - // Only for fast cofaces version to check the data structure for this feature is up to date + // Only for fast cofaces version to check the data structure for this feature is up to date using STree = Simplex_tree; STree st; diff --git a/src/Simplex_tree/test/simplex_tree_edge_expansion_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_edge_expansion_unit_test.cpp index 0bcc031767..618bde1b34 100644 --- a/src/Simplex_tree/test/simplex_tree_edge_expansion_unit_test.cpp +++ b/src/Simplex_tree/test/simplex_tree_edge_expansion_unit_test.cpp @@ -362,8 +362,3 @@ BOOST_AUTO_TEST_CASE(flag_expansion) { BOOST_CHECK(st.edge_with_same_filtration(st.find({1,5}))==st.find({1,5})); } } - - - - - diff --git a/src/Simplex_tree/test/simplex_tree_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_unit_test.cpp index d0dc0140ec..da2afb9ae1 100644 --- a/src/Simplex_tree/test/simplex_tree_unit_test.cpp +++ b/src/Simplex_tree/test/simplex_tree_unit_test.cpp @@ -36,7 +36,7 @@ typedef boost::mpl::list, Simplex_tree > list_of_tested_variants; template -void test_empty_simplex_tree(typeST& tst) { +void test_empty_simplex_tree(const typeST& tst) { typedef typename typeST::Vertex_handle Vertex_handle; const Vertex_handle DEFAULT_VERTEX_VALUE = Vertex_handle(- 1); BOOST_CHECK(tst.null_vertex() == DEFAULT_VERTEX_VALUE); @@ -44,7 +44,7 @@ void test_empty_simplex_tree(typeST& tst) { BOOST_CHECK(tst.num_simplices() == (size_t) 0); BOOST_CHECK(tst.is_empty()); BOOST_CHECK(tst.num_simplices_by_dimension() == std::vector()); - typename typeST::Siblings* STRoot = tst.root(); + const typename typeST::Siblings* STRoot = tst.root(); BOOST_CHECK(STRoot != nullptr); BOOST_CHECK(STRoot->oncles() == nullptr); BOOST_CHECK(STRoot->parent() == DEFAULT_VERTEX_VALUE); @@ -52,7 +52,7 @@ void test_empty_simplex_tree(typeST& tst) { } template -void test_iterators_on_empty_simplex_tree(typeST& tst) { +void test_iterators_on_empty_simplex_tree(const typeST& tst) { std::clog << "Iterator on vertices: " << std::endl; for (auto vertex : tst.complex_vertex_range()) { std::clog << "vertice:" << vertex << std::endl; @@ -137,7 +137,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_tree_from_file, typeST, list_of_tested_var } template -void test_simplex_tree_contains(typeST& simplexTree, typeSimplex& simplex, int pos) { +void test_simplex_tree_contains(const typeST& simplexTree, typeSimplex& simplex, int pos) { auto f_simplex = simplexTree.filtration_simplex_range().begin() + pos; std::clog << "test_simplex_tree_contains - filtration=" << simplexTree.filtration(*f_simplex) << "||" << simplex.second << std::endl; @@ -498,7 +498,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(NSimplexAndSubfaces_tree_insertion, typeST, list_o BOOST_CHECK(vertex == SimplexVector6[position]); position++; } - + /* Inserted simplex: */ /* 1 6 */ /* o---o */ @@ -593,7 +593,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(NSimplexAndSubfaces_tree_insertion, typeST, list_o } template -void test_cofaces(typeST& st, const std::vector& expected, int dim, const std::vector& res) { +void test_cofaces(const typeST& st, const std::vector& expected, int dim, const std::vector& res) { std::size_t nb_res = 0; if (dim == 0) { typename typeST::Cofaces_simplex_range stars = st.star_simplex_range(st.find(expected)); @@ -818,7 +818,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(copy_move_on_simplex_tree, typeST, list_of_tested_ std::clog << "Printing st - address = " << &st << std::endl; - // Copy constructor + // Copy constructor typeST st_copy = st; std::clog << "Printing a copy of st - address = " << &st_copy << std::endl; @@ -827,7 +827,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(copy_move_on_simplex_tree, typeST, list_of_tested_ // Check there is a new simplex tree reference BOOST_CHECK(&st != &st_copy); - // Move constructor + // Move constructor typeST st_move = std::move(st); std::clog << "Printing a move of st - address = " << &st_move << std::endl; @@ -836,14 +836,14 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(copy_move_on_simplex_tree, typeST, list_of_tested_ // Check there is a new simplex tree reference BOOST_CHECK(&st_move != &st_copy); BOOST_CHECK(&st_move != &st); - + typeST st_empty; // Check st has been emptied by the move BOOST_CHECK(st == st_empty); BOOST_CHECK(st.dimension() == -1); BOOST_CHECK(st.num_simplices() == 0); BOOST_CHECK(st.num_vertices() == (size_t)0); - + std::clog << "Printing st once again- address = " << &st << std::endl; } From f8b89120c6fc6a1ada2da9aca1258a4a1392dd08 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Fri, 18 Oct 2024 12:19:20 +0200 Subject: [PATCH 09/23] New unitary test for simplex tree constness --- .../test/simplex_tree_constness_unit_test.cpp | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp diff --git a/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp new file mode 100644 index 0000000000..d08da15e39 --- /dev/null +++ b/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp @@ -0,0 +1,129 @@ +/* This file is part of the Gudhi Library - https://gudhi.inria.fr/ - which is released under MIT. + * See file LICENSE or go to https://gudhi.inria.fr/licensing/ for full license details. + * Author(s): Vincent Rouvreau + * + * Copyright (C) 2024 Inria + * + * Modification(s): + * - YYYY/MM Author: Description of the modification + */ + +#define BOOST_TEST_DYN_LINK +#define BOOST_TEST_MODULE "simplex_tree_constness" +#include + +#include "gudhi/Simplex_tree.h" + +using namespace Gudhi; + +template +void test_simplex_tree_constness(const Simplex_tree& const_stree) { + Simplex_tree copy_operator = const_stree; + Simplex_tree copy_ctor(const_stree); + BOOST_CHECK(copy_operator == const_stree); + BOOST_CHECK(const_stree == copy_ctor); + + std::clog << "********************************************************************\n"; + std::clog << "* The complex contains " << const_stree.num_simplices() << " simplices, " << const_stree.num_vertices(); + std::clog << " vertices - dimension " << const_stree.dimension() << "\n"; + + std::clog << "* num_simplices_by_dimension = "; + for (auto simplices_by_dim: const_stree.num_simplices_by_dimension()) + std::clog << simplices_by_dim << ","; + std::clog << "\n"; + + std::clog << "* filtration_simplex_range\n"; + for (auto sh : const_stree.filtration_simplex_range()) { + std::clog << " " + << "[" << const_stree.filtration(sh) << "] "; + for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; + std::clog << std::endl; + } + std::clog << "* complex_simplex_range\n"; + for (auto sh : const_stree.complex_simplex_range()) { + std::clog << " " + << "[" << const_stree.filtration(sh) << "] "; + for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; + std::clog << std::endl; + } + std::clog << "* skeleton_simplex_range in dim " << const_stree.dimension() << "\n"; + for (auto sh : const_stree.skeleton_simplex_range(const_stree.dimension())) { + std::clog << " " + << "[" << const_stree.filtration(sh) << "] "; + for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; + std::clog << std::endl; + } + std::clog << "* star_simplex_range of {0, 1, 6}\n"; + for (auto sh : const_stree.star_simplex_range(const_stree.find({0, 1, 6}))) { + std::clog << " " + << "[" << const_stree.filtration(sh) << "] "; + for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; + std::clog << std::endl; + } + std::clog << "* cofaces_simplex_range of {0, 1, 6} in codimension 1\n"; + for (auto sh : const_stree.cofaces_simplex_range(const_stree.find({0, 1, 6}), 1)) { + std::clog << " " + << "[" << const_stree.filtration(sh) << "] "; + for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; + std::clog << std::endl; + } + std::clog << "* boundary_simplex_range of {0, 1, 6}\n"; + for (auto sh : const_stree.boundary_simplex_range(const_stree.find({0, 1, 6}))) { + std::clog << " " + << "[" << const_stree.filtration(sh) << "] "; + for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; + std::clog << std::endl; + } + std::clog << "* boundary_opposite_vertex_simplex_range of {0, 1, 6}\n"; + for (auto face_opposite_vertex : const_stree.boundary_opposite_vertex_simplex_range(const_stree.find({0, 1, 6}))) { + auto sh = face_opposite_vertex.first; + std::clog << " " + << "[" << const_stree.filtration(sh) << "] "; + for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; + std::clog << std::endl; + } + std::clog << "* Hasse diagram\n"; + const_stree.print_hasse(std::cout); + std::clog << "* vertex_with_same_filtration of {5, 3}" + << const_stree.vertex_with_same_filtration(const_stree.find({5, 3})) << "\n"; + std::clog << "* edge_with_same_filtration of {0, 1, 2}\n"; + { + auto sh = const_stree.edge_with_same_filtration(const_stree.find({0, 1, 2})); + for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; + std::clog << std::endl; + } + std::clog << "* minimal_simplex_with_same_filtration of {0, 1, 2}\n"; + { + auto sh = const_stree.minimal_simplex_with_same_filtration(const_stree.find({0, 1, 2})); + for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; + std::clog << std::endl; + } + std::clog << "* serialization size = " << const_stree.get_serialization_size() << "\n"; + std::clog << "* operator<<\n"; + std::clog << const_stree; +} + +BOOST_AUTO_TEST_CASE(const_simplex_tree) { + 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 o */ + /* 2 0 3\X/4 8 */ + /* o */ + /* 5 */ + /* */ + + // Required before browsing through filtration values + st.initialize_filtration(); + + test_simplex_tree_constness(st); + +} From 2d44573bb2cc706c4913cb8df744e1eacdc1a4cf Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Wed, 23 Oct 2024 09:15:58 +0200 Subject: [PATCH 10/23] comment review: redundant comment line --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 3d35884f95..f85b1bf185 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -1063,8 +1063,7 @@ class Simplex_tree { auto simplex_one = insertion_result.first; bool one_is_new = insertion_result.second; if (one_is_new) { - // update extra data structures in the insertion is successful - // Only required when insertion is successful + // update extra data structures if the insertion is successful update_simplex_tree_after_node_insertion(insertion_result.first); } else { if (filtration(simplex_one) > filt) { From 99b37a357f6c4faf4a006bdd33af5c9173d80800 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Wed, 23 Oct 2024 09:37:56 +0200 Subject: [PATCH 11/23] code review: const/non const version of simplex_data --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 9 ++++++- .../test/simplex_tree_constness_unit_test.cpp | 26 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index f85b1bf185..9f5fdc9e0e 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -716,7 +716,14 @@ class Simplex_tree { } /** \brief Returns the extra data stored in a simplex. */ - static Simplex_data& simplex_data(Simplex_handle sh) { + Simplex_data& simplex_data(Simplex_handle sh) { + GUDHI_CHECK(sh != null_simplex(), + std::invalid_argument("Simplex_tree::simplex_data - no data associated to null_simplex")); + return _to_node_it(sh)->second.data(); + } + + /** \brief Returns the extra data stored in a simplex. */ + const Simplex_data simplex_data(Simplex_handle sh) const { GUDHI_CHECK(sh != null_simplex(), std::invalid_argument("Simplex_tree::simplex_data - no data associated to null_simplex")); return _to_node_it(sh)->second.data(); diff --git a/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp index d08da15e39..f69e3cd32c 100644 --- a/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp +++ b/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp @@ -8,6 +8,8 @@ * - YYYY/MM Author: Description of the modification */ +#include + #define BOOST_TEST_DYN_LINK #define BOOST_TEST_MODULE "simplex_tree_constness" #include @@ -127,3 +129,27 @@ BOOST_AUTO_TEST_CASE(const_simplex_tree) { test_simplex_tree_constness(st); } + +template +void test_simplex_tree_data_constness(const Simplex_tree& const_stree) { + std::clog << "* test_simplex_tree_data_constness\n"; + std::clog << "simplex_data({0, 1}) = " << const_stree.simplex_data(const_stree.find({0, 1})) << "\n"; + BOOST_CHECK(const_stree.simplex_data(const_stree.find({0, 1})) == std::string("{0, 1}")); + std::clog << "simplex_data({0, 1, 2}) = " << const_stree.simplex_data(const_stree.find({0, 1, 2})) << "\n"; + BOOST_CHECK(const_stree.simplex_data(const_stree.find({2, 1, 0})) == std::string("{0, 1, 2}")); +} + +struct Options_with_int_data : Simplex_tree_options_minimal { + typedef std::string Simplex_data; +}; + +BOOST_AUTO_TEST_CASE(const_simplex_data) { + Simplex_tree st; + st.insert_simplex_and_subfaces({0, 1}); + st.insert_simplex_and_subfaces({2, 1}); + st.insert_simplex_and_subfaces({0, 2}); + st.simplex_data(st.find({0, 1})) = std::string("{0, 1}"); + st.expansion(3); + st.simplex_data(st.find({0, 1, 2})) = std::string("{0, 1, 2}"); + test_simplex_tree_data_constness(st); +} From 21430260dc4bc2d139d3dd57d3571969ac4427a2 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Wed, 23 Oct 2024 09:42:26 +0200 Subject: [PATCH 12/23] code review: Remove _cofaces_simplex_range that was useless --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 9f5fdc9e0e..419575972d 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -1261,11 +1261,6 @@ class Simplex_tree { * SimplexTreeOptions::link_nodes_by_label is true. */ Cofaces_simplex_range cofaces_simplex_range(const Simplex_handle simplex, int codimension) const{ - return _cofaces_simplex_range(simplex, codimension); - } - - private: - Cofaces_simplex_range _cofaces_simplex_range(const Simplex_handle simplex, int codimension) const { // codimension must be positive or null integer assert(codimension >= 0); @@ -1294,6 +1289,7 @@ class Simplex_tree { } } + private: /** \brief Returns true iff the list of vertices of sh1 * is smaller than the list of vertices of sh2 w.r.t. * lexicographic order on the lists read in reverse. From 0f8a7b8c673d2b1c8f5349e630a224c52823abe6 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Wed, 23 Oct 2024 09:44:49 +0200 Subject: [PATCH 13/23] code review: begin1 and end1 are now Const_dictionary_it --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 419575972d..5384e7c0ef 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -1775,7 +1775,7 @@ class Simplex_tree { * and assigns the maximal possible Filtration_value to the Nodes. */ template static void intersection(std::vector >& intersection, - Dictionary_it begin1, Dictionary_it end1, + Const_dictionary_it begin1, Const_dictionary_it end1, Const_dictionary_it begin2, Const_dictionary_it end2, Filtration_value filtration_) { if (begin1 == end1 || begin2 == end2) From f8de11a3d13b79d85c82a06ed960992fba3dbd6d Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Wed, 23 Oct 2024 10:54:38 +0200 Subject: [PATCH 14/23] code review: DRY for nodes_by_label --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 5384e7c0ef..b1907d960a 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -2378,17 +2378,15 @@ class Simplex_tree { std::unordered_map nodes_label_to_list_; List_max_vertex* nodes_by_label(Vertex_handle v) { - if constexpr (Options::link_nodes_by_label) { - auto it_v = nodes_label_to_list_.find(v); - if (it_v != nodes_label_to_list_.end()) { - return &(it_v->second); - } else { - return nullptr; - } - } - return nullptr; + // Scott Meyers in Effective C++ 3rd Edition. On page 23, Item 3: a non const method can safely call a const one + // Doing it the other way is not safe + return const_cast(_nodes_by_label(v)); } List_max_vertex const* nodes_by_label(Vertex_handle v) const { + return _nodes_by_label(v); + } + + List_max_vertex const* _nodes_by_label(Vertex_handle v) const { if constexpr (Options::link_nodes_by_label) { auto it_v = nodes_label_to_list_.find(v); if (it_v != nodes_label_to_list_.end()) { From da24a134b82d021593b21d49fae70a9321e237f8 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Wed, 23 Oct 2024 19:13:07 +0200 Subject: [PATCH 15/23] Also test read only for_each_simplex when const --- .../test/simplex_tree_constness_unit_test.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp index f69e3cd32c..b3fd2c4304 100644 --- a/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp +++ b/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp @@ -103,6 +103,14 @@ void test_simplex_tree_constness(const Simplex_tree& const_stree) { std::clog << "* serialization size = " << const_stree.get_serialization_size() << "\n"; std::clog << "* operator<<\n"; std::clog << const_stree; + + std::clog << "* for_each_simplex\n"; + const_stree.for_each_simplex([&](auto sh, int dim) { + std::clog << "dim = " << dim << " - "; + for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; + std::clog << std::endl; + }); + } BOOST_AUTO_TEST_CASE(const_simplex_tree) { From 94ca0d9b8523297c891383864d9625b147cad555 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Thu, 24 Oct 2024 11:48:14 +0200 Subject: [PATCH 16/23] [skip ci] Add some modification explanation as comment --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index b1907d960a..3f68032621 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -11,6 +11,7 @@ * - 2023/05 Hannah Schreiber: Factorization of expansion methods * - 2023/08 Hannah Schreiber (& Clément Maria): Add possibility of stable simplex handles. * - 2024/08 Hannah Schreiber: Addition of customizable copy constructor. + * - 2024/10 Hannah Schreiber: Const version of the Simplex_tree * - YYYY/MM Author: Description of the modification */ From 4277a64b2cd91796be8f51c62ac31a3ba2338669 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Tue, 5 Nov 2024 17:04:28 +0100 Subject: [PATCH 17/23] code review: std::as_const(*this) to avoid redirection --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 3f68032621..905a7f1f69 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -2381,13 +2381,10 @@ class Simplex_tree { List_max_vertex* nodes_by_label(Vertex_handle v) { // Scott Meyers in Effective C++ 3rd Edition. On page 23, Item 3: a non const method can safely call a const one // Doing it the other way is not safe - return const_cast(_nodes_by_label(v)); - } - List_max_vertex const* nodes_by_label(Vertex_handle v) const { - return _nodes_by_label(v); + return const_cast(std::as_const(*this).nodes_by_label(v)); } - List_max_vertex const* _nodes_by_label(Vertex_handle v) const { + List_max_vertex const* nodes_by_label(Vertex_handle v) const { if constexpr (Options::link_nodes_by_label) { auto it_v = nodes_label_to_list_.find(v); if (it_v != nodes_label_to_list_.end()) { From cbbdc8ae87e06e34be0cb9787c29915472ec01b0 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Tue, 5 Nov 2024 17:09:18 +0100 Subject: [PATCH 18/23] code review: simplex_data should return a const ref --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 905a7f1f69..4f57225a8e 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -724,7 +724,7 @@ class Simplex_tree { } /** \brief Returns the extra data stored in a simplex. */ - const Simplex_data simplex_data(Simplex_handle sh) const { + const Simplex_data& simplex_data(Simplex_handle sh) const { GUDHI_CHECK(sh != null_simplex(), std::invalid_argument("Simplex_tree::simplex_data - no data associated to null_simplex")); return _to_node_it(sh)->second.data(); From 3d21b0cb21db71c5647d2444854a672eebf80a9d Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Tue, 5 Nov 2024 17:55:45 +0100 Subject: [PATCH 19/23] code review: clear_filtration is now const. Mark methods that are not thread-safe --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 43 ++++++++++++++----- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 4f57225a8e..a97f0067c4 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -346,7 +346,10 @@ class Simplex_tree { * * The filtration must be valid. If the filtration has not been initialized yet, the * method initializes it (i.e. order the simplices). If the complex has changed since the last time the filtration - * was initialized, please call `clear_filtration()` or `initialize_filtration()` to recompute it. */ + * was initialized, please call `clear_filtration()` or `initialize_filtration()` to recompute it. + * + * @note Not thread safe + */ Filtration_simplex_range const& filtration_simplex_range(Indexing_tag = Indexing_tag()) const { maybe_initialize_filtration(); return filtration_vect_; @@ -676,6 +679,8 @@ class Simplex_tree { /** \brief Returns the simplex that has index idx in the filtration. * * The filtration must be initialized. + * + * @note Not thread safe */ Simplex_handle simplex(Simplex_key idx) const { return filtration_vect_[idx]; @@ -810,15 +815,20 @@ class Simplex_tree { return dimension(self_siblings(sh)); } - /** \brief Returns an upper bound on the dimension of the simplicial complex. */ + /** \brief Returns an upper bound on the dimension of the simplicial complex. + * + * @note Not thread safe + */ int upper_bound_dimension() const { return dimension_; } /** \brief Returns the dimension of the simplicial complex. - \details This function is not constant time because it can recompute dimension if required (can be triggered by - `remove_maximal_simplex()` or `prune_above_filtration()`). - */ + * \details This function is not constant time because it can recompute dimension if required (can be triggered by + * `remove_maximal_simplex()` or `prune_above_filtration()`). + * + * @note Not thread safe + */ int dimension() const { if (dimension_to_be_lowered_) lower_upper_bound_dimension(); @@ -1142,7 +1152,10 @@ class Simplex_tree { * It always recomputes the cache, even if one already exists. * * Any insertion, deletion or change of filtration value invalidates this cache, - * which can be cleared with clear_filtration(). */ + * which can be cleared with clear_filtration(). + * + * @note Not thread safe + */ void initialize_filtration(bool ignore_infinite_values = false) const { filtration_vect_.clear(); filtration_vect_.reserve(num_simplices()); @@ -1170,7 +1183,10 @@ class Simplex_tree { } /** \brief Initializes the filtration cache if it isn't initialized yet. * - * Automatically called by filtration_simplex_range(). */ + * Automatically called by filtration_simplex_range(). + * + * @note Not thread safe + */ void maybe_initialize_filtration() const { if (filtration_vect_.empty()) { initialize_filtration(); @@ -1179,8 +1195,11 @@ class Simplex_tree { /** \brief Clears the filtration cache produced by initialize_filtration(). * * Useful when initialize_filtration() has already been called and we perform an operation - * (say an insertion) that invalidates the cache. */ - void clear_filtration() { + * (say an insertion) that invalidates the cache. + * + * @note Not thread safe + */ + void clear_filtration() const { filtration_vect_.clear(); } @@ -1926,9 +1945,13 @@ class Simplex_tree { * Each row in the file correspond to a simplex. A line is written: * dim idx_1 ... idx_k fil where dim is the dimension of the simplex, * idx_1 ... idx_k are the row index (starting from 0) of the simplices of the boundary - * of the simplex, and fil is its filtration value. */ + * of the simplex, and fil is its filtration value. + * + * @note Not thread safe + */ void print_hasse(std::ostream& os) const { os << num_simplices() << " " << std::endl; + // TODO: should use complex_simplex_range ? for (auto sh : filtration_simplex_range()) { os << dimension(sh) << " "; for (auto b_sh : boundary_simplex_range(sh)) { From 38a751fb1789305a4376ded2df9ecf6576864368 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Tue, 12 Nov 2024 10:30:27 +0100 Subject: [PATCH 20/23] code review: add some tests for contiguous_vertices, simplex and complex_vertex_range --- .../test/simplex_tree_constness_unit_test.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp index b3fd2c4304..c6a6848b73 100644 --- a/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp +++ b/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp @@ -41,6 +41,14 @@ void test_simplex_tree_constness(const Simplex_tree& const_stree) { for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; std::clog << std::endl; } + std::clog << "* loop on simplex\n"; + for (std::size_t idx = 0; idx < const_stree.num_simplices(); idx++) { + auto sh = const_stree.simplex(idx); + std::clog << " " + << "[" << const_stree.filtration(sh) << "] "; + for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; + std::clog << std::endl; + } std::clog << "* complex_simplex_range\n"; for (auto sh : const_stree.complex_simplex_range()) { std::clog << " " @@ -48,6 +56,10 @@ void test_simplex_tree_constness(const Simplex_tree& const_stree) { for (auto vertex : const_stree.simplex_vertex_range(sh)) std::clog << "(" << vertex << ")"; std::clog << std::endl; } + std::clog << "* complex_vertex_range\n"; + std::clog << " "; + for (auto vertex : const_stree.complex_vertex_range()) std::clog << "(" << vertex << ")"; + std::clog << std::endl; std::clog << "* skeleton_simplex_range in dim " << const_stree.dimension() << "\n"; for (auto sh : const_stree.skeleton_simplex_range(const_stree.dimension())) { std::clog << " " @@ -111,6 +123,7 @@ void test_simplex_tree_constness(const Simplex_tree& const_stree) { std::clog << std::endl; }); + BOOST_CHECK(const_stree.contiguous_vertices()); } BOOST_AUTO_TEST_CASE(const_simplex_tree) { From 0d500bcc7dd0d86be21b68bd740824cd31fedf22 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Tue, 12 Nov 2024 10:36:25 +0100 Subject: [PATCH 21/23] code review: prefer the use of assign_filtration(sh, filt) --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index a97f0067c4..6d423dc5b2 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -723,9 +723,9 @@ class Simplex_tree { /** \brief Returns the extra data stored in a simplex. */ Simplex_data& simplex_data(Simplex_handle sh) { - GUDHI_CHECK(sh != null_simplex(), - std::invalid_argument("Simplex_tree::simplex_data - no data associated to null_simplex")); - return _to_node_it(sh)->second.data(); + // Scott Meyers in Effective C++ 3rd Edition. On page 23, Item 3: a non const method can safely call a const one + // Doing it the other way is not safe + return const_cast(std::as_const(*this).simplex_data(sh)); } /** \brief Returns the extra data stored in a simplex. */ @@ -2024,7 +2024,7 @@ class Simplex_tree { if (!(sh->second.filtration() >= max_filt_border_value)) { // Store the filtration modification information modified = true; - _to_node_it(sh)->second.assign_filtration(max_filt_border_value); + assign_filtration(sh, max_filt_border_value); } }; // Loop must be from the end to the beginning, as higher dimension simplex are always on the left part of the tree From 85efd97b21fa89db21fa6fa46ad1dc97763d5846 Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Tue, 12 Nov 2024 17:03:09 +0100 Subject: [PATCH 22/23] code review: _to_node_it, simplex_data and self_siblings are no more static with a const/non const version if necessary --- src/Simplex_tree/include/gudhi/Simplex_tree.h | 25 +++++++++++++------ .../Simplex_tree_node_explicit_storage.h | 1 + 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 6d423dc5b2..08dbacac82 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -662,8 +662,9 @@ class Simplex_tree { return sh->second.filtration(); } - static Dictionary_it _to_node_it(Simplex_handle sh){ - return const_cast(self_siblings(sh))->to_non_const_it(sh); + // Transform a Const_dictionary_it into a Dictionary_it + Dictionary_it _to_node_it(Simplex_handle sh) { + return self_siblings(sh)->to_non_const_it(sh); } public: @@ -723,16 +724,16 @@ class Simplex_tree { /** \brief Returns the extra data stored in a simplex. */ Simplex_data& simplex_data(Simplex_handle sh) { - // Scott Meyers in Effective C++ 3rd Edition. On page 23, Item 3: a non const method can safely call a const one - // Doing it the other way is not safe - return const_cast(std::as_const(*this).simplex_data(sh)); + GUDHI_CHECK(sh != null_simplex(), + std::invalid_argument("Simplex_tree::simplex_data - no data associated to null_simplex")); + return _to_node_it(sh)->second.data(); } /** \brief Returns the extra data stored in a simplex. */ const Simplex_data& simplex_data(Simplex_handle sh) const { GUDHI_CHECK(sh != null_simplex(), std::invalid_argument("Simplex_tree::simplex_data - no data associated to null_simplex")); - return _to_node_it(sh)->second.data(); + return sh->second.data(); } /** \brief Returns a Vertex_handle different from all Vertex_handles associated @@ -1118,13 +1119,21 @@ class Simplex_tree { /** Returns the Siblings containing a simplex.*/ template - static Siblings const* self_siblings(SimplexHandle sh) { + Siblings const* self_siblings(SimplexHandle sh) const { if (sh->second.children()->parent() == sh->first) return sh->second.children()->oncles(); else return sh->second.children(); } + /** Returns the Siblings containing a simplex.*/ + template + Siblings* self_siblings(SimplexHandle sh) { + // Scott Meyers in Effective C++ 3rd Edition. On page 23, Item 3: a non const method can safely call a const one + // Doing it the other way is not safe + return const_cast(std::as_const(*this).self_siblings(sh)); + } + public: /** Returns a pointer to the root nodes of the simplex tree. */ Siblings* root() { return &root_; } @@ -1590,7 +1599,7 @@ class Simplex_tree { { Node& node_u = static_cast(node_as_hook); //corresponding node, has label u Simplex_handle sh_u = simplex_handle_from_node(node_u); - Siblings* sib_u = const_cast(self_siblings(sh_u)); + Siblings* sib_u = self_siblings(sh_u); if (sib_u->members().find(v) != sib_u->members().end()) { //v is the label of a sibling of node_u int curr_dim = dimension(sib_u); if (dim_max == -1 || curr_dim < dim_max){ diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_node_explicit_storage.h b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_node_explicit_storage.h index 0cce87866f..05bb12b92e 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_node_explicit_storage.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_node_explicit_storage.h @@ -68,6 +68,7 @@ struct GUDHI_EMPTY_BASE_CLASS_OPTIMIZATION Simplex_tree_node_explicit_storage } Simplex_data& data() { return boost::empty_value::get(); } + const Simplex_data& data() const { return boost::empty_value::get(); } private: Siblings * children_; From 323e11967df6e2bf751710c58ca80c2f3306d2bf Mon Sep 17 00:00:00 2001 From: Vincent Rouvreau Date: Tue, 12 Nov 2024 17:23:05 +0100 Subject: [PATCH 23/23] code review: add const test for has_children and root() --- src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp index c6a6848b73..5aa18ed233 100644 --- a/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp +++ b/src/Simplex_tree/test/simplex_tree_constness_unit_test.cpp @@ -124,6 +124,8 @@ void test_simplex_tree_constness(const Simplex_tree& const_stree) { }); BOOST_CHECK(const_stree.contiguous_vertices()); + BOOST_CHECK(const_stree.has_children(const_stree.find({0, 1}))); + [[maybe_unused]] auto sh = const_stree.root(); // -> OK } BOOST_AUTO_TEST_CASE(const_simplex_tree) {