diff --git a/src/Simplex_tree/concept/FiltrationValue.h b/src/Simplex_tree/concept/FiltrationValue.h index a962a980b..81adaaaab 100644 --- a/src/Simplex_tree/concept/FiltrationValue.h +++ b/src/Simplex_tree/concept/FiltrationValue.h @@ -44,8 +44,9 @@ struct FiltrationValue { /** * @brief Given two filtration values at which a simplex exists, computes the minimal union of births generating * a lifetime including those two values. The result is stored in the first parameter. - * The overload for native arithmetic types like `double` or `int` is already implemented as the minimum of the - * two given values. + * The overload for arithmetic types like `double` or `int` is already implemented as the minimum of the + * two given values and can also be used for non native arithmetic types like `CGAL::Gmpq` as long as it has an + * `operator<`. The overload is available with @ref Gudhi::unify in `simplex_tree_options.h`. * * For a k-critical filtration, FiltrationValue should be able to store an union of values (corresponding to the * different births of a same simplex) and this method adds the values of @p f2 in @p f1 and removes the values @@ -59,9 +60,10 @@ struct FiltrationValue { friend bool unify(FiltrationValue& f1, const FiltrationValue& f2); /** - * @brief Given two filtration values, stores in the first value the greatest common upper bound of the two values. - * The overload for native arithmetic types like `double` or `int` is already implemented as the maximum of the two - * given values. + * @brief Given two filtration values, stores in the first value the lowest common upper bound of the two values. + * The overload for arithmetic types like `double` or `int` is already implemented as the maximum of the two + * given values and can also be used for non native arithmetic types like `CGAL::Gmpq` as long as it has an + * `operator<`. The overload is available with @ref Gudhi::intersect in `simplex_tree_options.h`. * * @return True if and only if the values in @p f1 were actually modified. */ diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h index 0c967a2cf..8b0d94efb 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h @@ -1194,15 +1194,6 @@ class Simplex_tree { Simplex_tree * st_; }; - void fill_filtration_cache(bool ignore_infinite_values){ - filtration_vect_.clear(); - filtration_vect_.reserve(num_simplices()); - for (Simplex_handle sh : complex_simplex_range()) { - if (ignore_infinite_values && filtration(sh) == Filtration_simplex_base_real::get_infinity()) continue; - filtration_vect_.push_back(sh); - } - } - public: /** \brief Initializes the filtration cache, i.e. sorts the * simplices according to their order in the filtration. @@ -1216,26 +1207,9 @@ class Simplex_tree { * which can be cleared with clear_filtration(). */ void initialize_filtration(bool ignore_infinite_values = false) { - fill_filtration_cache(ignore_infinite_values); - - /* We use stable_sort here because with libstdc++ it is faster than sort. - * is_before_in_totally_ordered_filtration is now a total order, but we used to call - * stable_sort for the following heuristic: - * The use of a depth-first traversal of the simplex tree, provided by - * complex_simplex_range(), combined with a stable sort is meant to - * optimize the order of simplices with same filtration value. The - * heuristic consists in inserting the cofaces of a simplex as soon as - * possible. - */ -#ifdef GUDHI_USE_TBB - tbb::parallel_sort(filtration_vect_.begin(), filtration_vect_.end(), is_before_in_totally_ordered_filtration(this)); -#else - std::stable_sort(filtration_vect_.begin(), filtration_vect_.end(), is_before_in_totally_ordered_filtration(this)); -#endif + initialize_filtration(is_before_in_totally_ordered_filtration(this), ignore_infinite_values); } - // TODO: is there a possibility to give `is_before_in_totally_ordered_filtration(this)()` as default parameter for - // `is_before_in_filtration` to avoid having two methods? /** * @brief Initializes the filtration cache, i.e. sorts the simplices according to the specified order. * The given order has to totally order all simplices in the simplex tree such that the resulting order is a valid @@ -1264,7 +1238,12 @@ class Simplex_tree { */ template void initialize_filtration(Comparator&& is_before_in_filtration, bool ignore_infinite_values = false) { - fill_filtration_cache(ignore_infinite_values); + filtration_vect_.clear(); + filtration_vect_.reserve(num_simplices()); + for (Simplex_handle sh : complex_simplex_range()) { + if (ignore_infinite_values && filtration(sh) == Filtration_simplex_base_real::get_infinity()) continue; + filtration_vect_.push_back(sh); + } #ifdef GUDHI_USE_TBB tbb::parallel_sort(filtration_vect_.begin(), filtration_vect_.end(), is_before_in_filtration); @@ -2300,7 +2279,7 @@ class Simplex_tree { * values are actually modified. The function `decode_extended_filtration()` * retrieves the original values and outputs the extended simplex type. * - * @warning Currently only works for @ref SimplexTreeOptions::Filtration_value which are native + * @warning Currently only works for @ref SimplexTreeOptions::Filtration_value which are * float types like `float` or `double`. * * @exception std::invalid_argument In debug mode if the Simplex tree contains a vertex with the largest diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree/simplex_tree_options.h b/src/Simplex_tree/include/gudhi/Simplex_tree/simplex_tree_options.h index c23759f06..c3fb87cbf 100644 --- a/src/Simplex_tree/include/gudhi/Simplex_tree/simplex_tree_options.h +++ b/src/Simplex_tree/include/gudhi/Simplex_tree/simplex_tree_options.h @@ -15,7 +15,6 @@ #include #include // void_t -#include // std::min #include // std::isnan namespace Gudhi { @@ -102,19 +101,17 @@ template struct Get_simplex_data_type> { typedef typename O::Simplex_data type; }; /** - * @private * @brief Given two filtration values at which a simplex exists, stores in the first value the minimal union of births * generating a lifetime including those two values. - * This is the overload for when `Filtration_value` is a native arithmetic type, like double, int etc. + * This is the overload for when `Filtration_value` is a arithmetic type, like double, int etc. * Because the filtration values are totally ordered then, the union is simply the minimum of the two values. * * NaN values are not supported. */ -template > > +template bool unify(Arithmetic_filtration_value& f1, Arithmetic_filtration_value f2) { - if (f1 > f2){ + if (f2 < f1){ f1 = f2; return true; } @@ -122,14 +119,12 @@ bool unify(Arithmetic_filtration_value& f1, Arithmetic_filtration_value f2) } /** - * @private * @brief Given two filtration values, stores in the first value the greatest common upper bound of the two values. * If a filtration value has value `NaN`, it should be considered as the lowest value possible. - * This is the overload for when `Filtration_value` is a native arithmetic type, like double, float, int etc. + * This is the overload for when `Filtration_value` is a arithmetic type, like double, float, int etc. * Because the filtration values are totally ordered then, the upper bound is always the maximum of the two values. */ -template > > +template bool intersect(Arithmetic_filtration_value& f1, Arithmetic_filtration_value f2) { if constexpr (std::is_floating_point_v) {