Skip to content

Commit

Permalink
vineyard fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@hschreiber
hschreiber committed Apr 19, 2024
1 parent 9a64c20 commit aca2e02
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 102 deletions.
47 changes: 25 additions & 22 deletions src/Persistence_matrix/include/gudhi/Persistence_matrix/chain_matrix.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ class Chain_matrix : public Master_matrix::Matrix_dimension_option,
* @ref PersistenceMatrixOptions::has_column_pairings is also true, the comparators are ignored and
* the current barcode is used to compare birth and deaths. Therefore it is useless to provide them in those cases.
*
* @tparam EventComparatorFunction Method of the form: ( @ref pos_index, @ref pos_index ) -> bool.
* @tparam BirthComparatorFunction Type of the birth comparator: (@ref pos_index, @ref pos_index) -> bool
* @tparam DeathComparatorFunction Type of the death comparator: (@ref pos_index, @ref pos_index) -> bool
* @param operators Pointer to the field operators.
* @param cellConstructor Pointer to the cell factory.
* @param birthComparator Method taking two @ref PosIdx indices as input and returning true if and only if
Expand All @@ -131,11 +132,11 @@ class Chain_matrix : public Master_matrix::Matrix_dimension_option,
* the second one with respect to some self defined order. It is used while swapping two positive but paired
* columns.
*/
template <typename EventComparatorFunction>
template <typename BirthComparatorFunction, typename DeathComparatorFunction>
Chain_matrix(Field_operators* operators,
Cell_constructor* cellConstructor,
EventComparatorFunction&& birthComparator,
EventComparatorFunction&& deathComparator);
const BirthComparatorFunction& birthComparator,
const DeathComparatorFunction& deathComparator);
/**
* @brief Constructs a new matrix from the given ranges of @ref Matrix::cell_rep_type. Each range corresponds to a
* column (the order of the ranges are preserved). The content of the ranges is assumed to be sorted by increasing
Expand All @@ -146,7 +147,8 @@ class Chain_matrix : public Master_matrix::Matrix_dimension_option,
* @ref PersistenceMatrixOptions::has_column_pairings is also true, the comparators are ignored and
* the current barcode is used to compare birth and deaths. Therefore it is useless to provide them in those cases.
*
* @tparam EventComparatorFunction Method of the form: ( @ref pos_index, @ref pos_index ) -> bool.
* @tparam BirthComparatorFunction Type of the birth comparator: (@ref pos_index, @ref pos_index) -> bool
* @tparam DeathComparatorFunction Type of the death comparator: (@ref pos_index, @ref pos_index) -> bool
* @tparam Boundary_type Range type for @ref Matrix::cell_rep_type ranges.
* Assumed to have a begin(), end() and size() method.
* @param orderedBoundaries Range of boundaries: @p orderedBoundaries is interpreted as a boundary matrix of a
Expand All @@ -158,7 +160,7 @@ class Chain_matrix : public Master_matrix::Matrix_dimension_option,
* All dimensions up to the maximal dimension of interest have to be present. If only a higher dimension is of
* interest and not everything should be stored, then use the @ref insert_boundary method instead
* (after creating the matrix with the @ref Chain_matrix(unsigned int, Field_operators*, Cell_constructor*,
* EventComparatorFunction&&, EventComparatorFunction&&) constructor preferably).
* const BirthComparatorFunction&, const DeathComparatorFunction&) constructor preferably).
* @param operators Pointer to the field operators.
* @param cellConstructor Pointer to the cell factory.
* @param birthComparator Method taking two @ref PosIdx indices as input and returning true if and only if
Expand All @@ -170,12 +172,12 @@ class Chain_matrix : public Master_matrix::Matrix_dimension_option,
* the second one with respect to some self defined order. It is used while swapping two positive but paired
* columns.
*/
template <typename EventComparatorFunction, class Boundary_type = boundary_type>
template <typename BirthComparatorFunction, typename DeathComparatorFunction, class Boundary_type = boundary_type>
Chain_matrix(const std::vector<Boundary_type>& orderedBoundaries,
Field_operators* operators,
Cell_constructor* cellConstructor,
EventComparatorFunction&& birthComparator,
EventComparatorFunction&& deathComparator);
const BirthComparatorFunction& birthComparator,
const DeathComparatorFunction& deathComparator);
/**
* @brief Constructs a new empty matrix and reserves space for the given number of columns.
*
Expand All @@ -184,7 +186,8 @@ class Chain_matrix : public Master_matrix::Matrix_dimension_option,
* @ref PersistenceMatrixOptions::has_column_pairings is also true, the comparators are ignored and
* the current barcode is used to compare birth and deaths. Therefore it is useless to provide them in those cases.
*
* @tparam EventComparatorFunction Method of the form: ( @ref pos_index, @ref pos_index ) -> bool.
* @tparam BirthComparatorFunction Type of the birth comparator: (@ref pos_index, @ref pos_index) -> bool
* @tparam DeathComparatorFunction Type of the death comparator: (@ref pos_index, @ref pos_index) -> bool
* @param numberOfColumns Number of columns to reserve space for.
* @param operators Pointer to the field operators.
* @param cellConstructor Pointer to the cell factory.
Expand All @@ -197,12 +200,12 @@ class Chain_matrix : public Master_matrix::Matrix_dimension_option,
* the second one with respect to some self defined order. It is used while swapping two positive but paired
* columns.
*/
template <typename EventComparatorFunction>
template <typename BirthComparatorFunction, typename DeathComparatorFunction>
Chain_matrix(unsigned int numberOfColumns,
Field_operators* operators,
Cell_constructor* cellConstructor,
EventComparatorFunction&& birthComparator,
EventComparatorFunction&& deathComparator);
const BirthComparatorFunction& birthComparator,
const DeathComparatorFunction& deathComparator);
/**
* @brief Copy constructor. If @p operators or @p cellConstructor is not a null pointer, its value is kept
* instead of the one in the copied matrix.
Expand Down Expand Up @@ -605,11 +608,11 @@ inline Chain_matrix<Master_matrix>::Chain_matrix(unsigned int numberOfColumns,
}

template <class Master_matrix>
template <typename EventComparatorFunction>
template <typename BirthComparatorFunction, typename DeathComparatorFunction>
inline Chain_matrix<Master_matrix>::Chain_matrix(Field_operators* operators,
Cell_constructor* cellConstructor,
EventComparatorFunction&& birthComparator,
EventComparatorFunction&& deathComparator)
const BirthComparatorFunction& birthComparator,
const DeathComparatorFunction& deathComparator)
: dim_opt(-1),
pair_opt(),
swap_opt(birthComparator, deathComparator),
Expand All @@ -621,12 +624,12 @@ inline Chain_matrix<Master_matrix>::Chain_matrix(Field_operators* operators,
{}

template <class Master_matrix>
template <typename EventComparatorFunction, class Boundary_type>
template <typename BirthComparatorFunction, typename DeathComparatorFunction, class Boundary_type>
inline Chain_matrix<Master_matrix>::Chain_matrix(const std::vector<Boundary_type>& orderedBoundaries,
Field_operators* operators,
Cell_constructor* cellConstructor,
EventComparatorFunction&& birthComparator,
EventComparatorFunction&& deathComparator)
const BirthComparatorFunction& birthComparator,
const DeathComparatorFunction& deathComparator)
: dim_opt(-1),
pair_opt(),
swap_opt(birthComparator, deathComparator),
Expand All @@ -648,12 +651,12 @@ inline Chain_matrix<Master_matrix>::Chain_matrix(const std::vector<Boundary_type
}

template <class Master_matrix>
template <typename EventComparatorFunction>
template <typename BirthComparatorFunction, typename DeathComparatorFunction>
inline Chain_matrix<Master_matrix>::Chain_matrix(unsigned int numberOfColumns,
Field_operators* operators,
Cell_constructor* cellConstructor,
EventComparatorFunction&& birthComparator,
EventComparatorFunction&& deathComparator)
const BirthComparatorFunction& birthComparator,
const DeathComparatorFunction& deathComparator)
: dim_opt(-1),
pair_opt(),
swap_opt(birthComparator, deathComparator),
Expand Down
19 changes: 6 additions & 13 deletions src/Persistence_matrix/include/gudhi/Persistence_matrix/chain_vine_swap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ struct Dummy_chain_vine_swap {
friend void swap([[maybe_unused]] Dummy_chain_vine_swap& d1, [[maybe_unused]] Dummy_chain_vine_swap& d2) {}

Dummy_chain_vine_swap() {}
template <typename EventComparatorFunction>
Dummy_chain_vine_swap([[maybe_unused]] EventComparatorFunction&& birthComparator,
[[maybe_unused]] EventComparatorFunction&& deathComparator) {}
template <typename BirthComparatorFunction, typename DeathComparatorFunction>
Dummy_chain_vine_swap([[maybe_unused]] const BirthComparatorFunction& birthComparator,
[[maybe_unused]] const DeathComparatorFunction& deathComparator) {}
};

/**
Expand Down Expand Up @@ -247,7 +247,6 @@ class Chain_vine_swap : public std::conditional<Master_matrix::Option_list::has_
/**
* @brief Constructor storing the given comparators.
*
* @tparam EventComparatorFunction Method of the form: ( @ref pos_index, @ref pos_index ) -> bool.
* @param birthComparator Method taking two @ref PosIdx indices as input and returning true if and only if
* the birth associated to the first position is strictly less than birth associated to
* the second one with respect to some self defined order. It is used while swapping two unpaired or
Expand Down Expand Up @@ -277,8 +276,7 @@ class Chain_vine_swap : public std::conditional<Master_matrix::Option_list::has_
* therefore skips a part of the case study.
*
* @param columnIndex1 @ref MatIdx index of the first face.
* @param columnIndex2 @ref MatIdx index of the second face. It is assumed that the @ref PosIdx of both only differs
* by one.
* @param columnIndex2 @ref MatIdx index of the second face.
* @return Let \f$ pos1 \f$ be the @ref PosIdx index of @p columnIndex1 and \f$ pos2 \f$ be the @ref PosIdx index of
* @p columnIndex2. The method returns the @ref MatIdx of the column which has now, after the swap, the @ref PosIdx
* \f$ max(pos1, pos2) \f$.
Expand All @@ -294,7 +292,7 @@ class Chain_vine_swap : public std::conditional<Master_matrix::Option_list::has_
*
* @param columnIndex1 @ref MatIdx index of the first face.
* @param columnIndex2 @ref MatIdx index of the second face. It is assumed that the @ref PosIdx of both only differs
* by one.
* by one if the barcode is maintained.
* @return Let \f$ pos1 \f$ be the @ref PosIdx index of @p columnIndex1 and \f$ pos2 \f$ be the @ref PosIdx index of
* @p columnIndex2. The method returns the @ref MatIdx of the column which has now, after the swap, the @ref PosIdx
* \f$ max(pos1, pos2) \f$.
Expand Down Expand Up @@ -371,11 +369,6 @@ template <class Master_matrix>
inline typename Chain_vine_swap<Master_matrix>::index Chain_vine_swap<Master_matrix>::vine_swap_with_z_eq_1_case(
index columnIndex1, index columnIndex2)
{
if constexpr (Master_matrix::Option_list::has_column_pairings) {
assert(CP::are_adjacent(_matrix()->get_pivot(columnIndex1), _matrix()->get_pivot(columnIndex2)) &&
"Columns to be swaped need to be adjacent in the 'real' matrix.");
}

const bool col1IsNeg = _is_negative_in_pair(columnIndex1);
const bool col2IsNeg = _is_negative_in_pair(columnIndex2);

Expand Down Expand Up @@ -585,7 +578,7 @@ inline typename Chain_vine_swap<Master_matrix>::index Chain_vine_swap<Master_mat
if constexpr (Master_matrix::Option_list::has_column_pairings) {
hasSmallerBirth = (CP::birth(col1.get_pivot()) < CP::birth(col2.get_pivot()));
} else {
hasSmallerBirth = birthComp_(columnIndex1, columnIndex2);
hasSmallerBirth = birthComp_(pairedIndex1, pairedIndex2);
}

if constexpr (Master_matrix::Option_list::has_column_pairings) {
Expand Down
Loading

0 comments on commit aca2e02

Please sign in to comment.