Skip to content

Commit

Permalink
Merge pull request #1145 from VincentRouvreau/const_simplex_tree_unit…
Browse files Browse the repository at this point in the history
…ary_tests

Const simplex tree unitary tests and fix operator== and !=
  • Loading branch information
VincentRouvreau authored Jan 24, 2025
2 parents f9f0cf3 + a7ca7d2 commit 5cdc175
Show file tree
Hide file tree
Showing 11 changed files with 442 additions and 192 deletions.
1 change: 1 addition & 0 deletions .github/code_conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* The classes and functions of a package should be in a sub-namespace of the `Gudhi` namespace. The sub-namespace names are in lowercase and use underscore separators. E.g. `Gudhi::package_name::`
* Concepts are named with camel case starting with uppercase. E.g. `PersistentHomology` for the concept of Persitence homology.
* Classes start with an uppercase letter and use underscore separators. E.g. `Skeleton_blocker_contractor`.
* Constant iterators are preferably named as `*_const_iterator`. E.g. `Vertex_const_iterator`.
* Member functions and free functions are in lowercase and use underscore separators. E.g. `int num_vertices()`.
* Constants and macros are in uppercase.
* Macros should begin with the prefix `GUDHI_`.
Expand Down
324 changes: 191 additions & 133 deletions src/Simplex_tree/include/gudhi/Simplex_tree.h

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

#include <boost/iterator/iterator_facade.hpp>

#include <vector>
#include <utility> // for std::pair

namespace Gudhi {
Expand Down Expand Up @@ -67,7 +66,7 @@ class Simplex_tree_simplex_vertex_iterator : public boost::iterator_facade<
sib_ = sib_->oncles();
}

Siblings * sib_;
Siblings const* sib_;
Vertex_handle v_;
};

Expand All @@ -93,7 +92,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),
Expand All @@ -102,7 +101,7 @@ class Simplex_tree_boundary_simplex_iterator : public boost::iterator_facade<
}

template<class SimplexHandle>
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),
Expand All @@ -111,7 +110,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) {
Expand Down Expand Up @@ -146,8 +145,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) {
Expand Down Expand Up @@ -180,9 +179,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.
Expand All @@ -206,7 +205,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),
Expand All @@ -215,7 +214,7 @@ class Simplex_tree_boundary_opposite_vertex_simplex_iterator : public boost::ite
}

template<class SimplexHandle>
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),
Expand All @@ -224,7 +223,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) {
Expand Down Expand Up @@ -258,8 +257,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) {
Expand Down Expand Up @@ -294,9 +293,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<Simplex_handle, Vertex_handle> 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
};

/*---------------------------------------------------------------------------*/
Expand All @@ -318,7 +317,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()) {
Expand Down Expand Up @@ -369,8 +368,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
Expand All @@ -394,7 +393,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),
Expand Down Expand Up @@ -450,8 +449,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_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,12 @@ struct GUDHI_EMPTY_BASE_CLASS_OPTIMIZATION Simplex_tree_node_explicit_storage
Siblings * children() {
return children_;
}
const Siblings * children() const {
return children_;
}

Simplex_data& data() { return boost::empty_value<Simplex_data>::get(); }
const Simplex_data& data() const { return boost::empty_value<Simplex_data>::get(); }

private:
Siblings * children_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@

#include <boost/container/flat_map.hpp>

#include <utility>
#include <vector>

namespace Gudhi {

/** \addtogroup simplex_tree
Expand All @@ -43,6 +40,7 @@ class Simplex_tree_siblings {
typedef typename SimplexTree::Node Node;
typedef MapContainer Dictionary;
typedef typename MapContainer::iterator Dictionary_it;
typedef typename MapContainer::const_iterator Dictionary_const_it;

/* Default constructor.*/
Simplex_tree_siblings()
Expand Down Expand Up @@ -74,8 +72,8 @@ class Simplex_tree_siblings {

/** \brief Inserts a Node in the set of siblings nodes.
*
* If already present, assigns the minimal filtration value
* between input filtration_value and the value already
* If already present, assigns the minimal filtration value
* between input filtration_value and the value already
* present in the node.
*/
void insert(Vertex_handle v, Filtration_value filtration_value) {
Expand All @@ -87,10 +85,16 @@ class Simplex_tree_siblings {
Dictionary_it find(Vertex_handle v) {
return members_.find(v);
}
Dictionary_const_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_;
Expand All @@ -100,6 +104,10 @@ class Simplex_tree_siblings {
return members_;
}

const Dictionary & members() const {
return members_;
}

size_t size() const {
return members_.size();
}
Expand All @@ -108,6 +116,10 @@ class Simplex_tree_siblings {
members_.erase(iterator);
}

Dictionary_it to_non_const_it(Dictionary_const_it it) {
return members_.erase(it, it);
}

Simplex_tree_siblings * oncles_;
Vertex_handle parent_;
Dictionary members_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Node&>(curr_hooks);
bool operator()(const typename SimplexTree::Hooks_simplex_base& curr_hooks) {
const Node& curr_node = static_cast<const Node&>(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);
Expand All @@ -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<is_coface, typename SimplexTree::List_max_vertex::iterator>
typedef boost::filter_iterator<is_coface, typename SimplexTree::List_max_vertex::const_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<Node&>(*it_);
const Node& curr_node = static_cast<const Node&>(*it_);
sh_ = st_->simplex_handle_from_node(curr_node);
}

Expand All @@ -128,15 +128,15 @@ class Simplex_tree_optimized_cofaces_rooted_subtrees_simplex_iterator
st_ = nullptr;
} //== end
else { // update sh_
Node& curr_node = static_cast<Node&>(*it_);
const Node& curr_node = static_cast<const Node&>(*it_);
sh_ = st_->simplex_handle_from_node(curr_node);
}
}

// 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_;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -265,9 +265,9 @@ 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<Siblings*> children_stack_;
std::vector<Siblings const*> children_stack_;
// true iff sh_ points to the root of a coface subtree
bool is_root_;
};
Expand Down
3 changes: 3 additions & 0 deletions src/Simplex_tree/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Loading

0 comments on commit 5cdc175

Please sign in to comment.