Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Const simplex tree unitary tests and fix operator== and != #1145

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
250757d
begining
hschreiber Aug 2, 2024
5ac7d58
const compiles
hschreiber Aug 12, 2024
f5f9922
Merge branch 'GUDHI:master' into const_simplex_tree
hschreiber Aug 12, 2024
587c03d
merge upstream + doc
hschreiber Aug 12, 2024
ef6c96c
Merge branch 'GUDHI:master' into const_simplex_tree
hschreiber Aug 27, 2024
a5e3e0f
upstream merge
hschreiber Oct 15, 2024
0820a27
Merge branch 'const_simplex_tree' of github.com:hschreiber/gudhi-deve…
hschreiber Oct 15, 2024
b7f4649
upstream merge
hschreiber Oct 15, 2024
e425049
restore Persistent_cohomology.h
hschreiber Oct 15, 2024
8067ba7
removal of non const ranges
hschreiber Oct 15, 2024
06834b9
removal of non const ranges
hschreiber Oct 15, 2024
8055160
Fix operator== and != for constness. Add some TU for constness
VincentRouvreau Oct 18, 2024
f8b8912
New unitary test for simplex tree constness
VincentRouvreau Oct 18, 2024
2d44573
comment review: redundant comment line
VincentRouvreau Oct 23, 2024
99b37a3
code review: const/non const version of simplex_data
VincentRouvreau Oct 23, 2024
2143026
code review: Remove _cofaces_simplex_range that was useless
VincentRouvreau Oct 23, 2024
0f8a7b8
code review: begin1 and end1 are now Const_dictionary_it
VincentRouvreau Oct 23, 2024
f8de11a
code review: DRY for nodes_by_label
VincentRouvreau Oct 23, 2024
da24a13
Also test read only for_each_simplex when const
VincentRouvreau Oct 23, 2024
94ca0d9
[skip ci] Add some modification explanation as comment
VincentRouvreau Oct 24, 2024
4277a64
code review: std::as_const(*this) to avoid redirection
VincentRouvreau Nov 5, 2024
cbbdc8a
code review: simplex_data should return a const ref
VincentRouvreau Nov 5, 2024
3d21b0c
code review: clear_filtration is now const. Mark methods that are not…
VincentRouvreau Nov 5, 2024
38a751f
code review: add some tests for contiguous_vertices, simplex and comp…
VincentRouvreau Nov 12, 2024
0d500bc
code review: prefer the use of assign_filtration(sh, filt)
VincentRouvreau Nov 12, 2024
85efd97
code review: _to_node_it, simplex_data and self_siblings are no more …
VincentRouvreau Nov 12, 2024
323e119
code review: add const test for has_children and root()
VincentRouvreau Nov 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 Const_dictionary_it;

/* Default constructor.*/
Simplex_tree_siblings()
Expand Down Expand Up @@ -87,10 +85,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_;
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(Const_dictionary_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
Loading