Skip to content

Commit

Permalink
Merge pull request #1153 from hschreiber/persistence_matrix
Browse files Browse the repository at this point in the history 
[Persistence Matrix] replacing static_cast(-1) by method
  • Loading branch information
@VincentRouvreau
VincentRouvreau authored Feb 3, 2025
2 parents 5cdc175 + 62b47a1 commit bf241cd
Show file tree
Hide file tree
Showing 29 changed files with 525 additions and 315 deletions.
16 changes: 9 additions & 7 deletions src/Persistence_matrix/concept/PersistenceMatrixColumn.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,18 +242,19 @@ class PersistenceMatrixColumn :

/**
* @brief Reorders the column with the given map of row indices. Also changes the column index stored in the
* entries if row access is enabled and @p columnIndex is not -1.
* entries if row access is enabled and @p columnIndex is not the @ref Matrix::get_null_value "null index".
*
* Only useful for @ref basematrix "base" and @ref boundarymatrix "boundary matrices" using lazy swaps.
*
* @tparam Row_index_map Map with an %at() method.
* @param valueMap Map such that `valueMap.at(i)` indicates the new row index of the entry
* at current row index `i`.
* @param columnIndex New @ref MatIdx column index of the column. If -1, the index does not change. Ignored if
* the row access is not enabled. Default value: -1.
* @param columnIndex New @ref MatIdx column index of the column. If @ref Matrix::get_null_value "null index",
* the index does not change. Ignored if the row access is not enabled.
* Default value: @ref Matrix::get_null_value "null index".
*/
template <class Row_index_map>
void reorder(const Row_index_map& valueMap, [[maybe_unused]] Index columnIndex = -1);
void reorder(const Row_index_map& valueMap, [[maybe_unused]] Index columnIndex = Matrix::get_null_value<Index>());
/**
* @brief Zeros/empties the column.
*
Expand All @@ -275,11 +276,12 @@ class PersistenceMatrixColumn :
void clear(ID_index rowIndex);

/**
* @brief Returns the row index of the pivot. If the column does not have a pivot, returns -1.
* @brief Returns the row index of the pivot. If the column does not have a pivot,
* returns @ref Matrix::get_null_value "null index".
*
* Only useful for @ref boundarymatrix "boundary" and @ref chainmatrix "chain matrices".
*
* @return Row index of the pivot or -1.
* @return Row index of the pivot or @ref Matrix::get_null_value "null index".
*/
ID_index get_pivot();
/**
Expand Down Expand Up @@ -429,7 +431,7 @@ class PersistenceMatrixColumn :
/**
* @brief Adds a copy of the given entry at the end of the column. It is therefore assumed that the row index
* of the entry is higher than the current pivot of the column. Not available for @ref chainmatrix "chain matrices"
* and is only needed for @ref rumatrix "RU matrices".
* and is only needed for @ref boundarymatrix "RU matrices".
*
* @param entry Entry to push back.
*/
Expand Down
31 changes: 20 additions & 11 deletions src/Persistence_matrix/include/gudhi/Matrix.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,15 @@ class Matrix {
*/
using Element = typename Field_operators::Element;
using Characteristic = typename Field_operators::Characteristic;


/**
* @brief Returns value from a type when not set.
*/
template <typename T>
static constexpr const T get_null_value() {
return -1;
}

/**
* @brief Type for a bar in the computed barcode. Stores the birth, death and dimension of the bar.
*/
Expand Down Expand Up @@ -367,7 +375,7 @@ class Matrix {
//purposely triggers operators() instead of operators(characteristic) as the "dummy" values for the different
//operators can be different from -1.
Column_zp_settings(Characteristic characteristic) : operators(), entryConstructor() {
if (characteristic != static_cast<Characteristic>(-1)) operators.set_characteristic(characteristic);
if (characteristic != get_null_value<Characteristic>()) operators.set_characteristic(characteristic);
}
Column_zp_settings(const Column_zp_settings& toCopy)
: operators(toCopy.operators.get_characteristic()), entryConstructor() {}
Expand Down Expand Up @@ -592,7 +600,7 @@ class Matrix {
* @ref set_characteristic before calling for the first time a method needing it. Ignored if
* @ref PersistenceMatrixOptions::is_z2 is true.
*/
Matrix(unsigned int numberOfColumns, Characteristic characteristic = static_cast<Characteristic>(-1));
Matrix(unsigned int numberOfColumns, Characteristic characteristic = Matrix::get_null_value<Characteristic>());
/**
* @brief Constructs a new empty matrix with the given comparator functions. Only available when those comparators
* are necessary.
Expand Down Expand Up @@ -671,7 +679,7 @@ class Matrix {
Matrix(unsigned int numberOfColumns,
const std::function<bool(Pos_index,Pos_index)>& birthComparator,
const std::function<bool(Pos_index,Pos_index)>& deathComparator,
Characteristic characteristic = static_cast<Characteristic>(-1));
Characteristic characteristic = Matrix::get_null_value<Characteristic>());
/**
* @brief Copy constructor.
*
Expand Down Expand Up @@ -765,7 +773,7 @@ class Matrix {
* chains used to reduce the boundary. Otherwise, nothing.
*/
template <class Boundary_range = Boundary>
Insertion_return insert_boundary(const Boundary_range& boundary, Dimension dim = -1);
Insertion_return insert_boundary(const Boundary_range& boundary, Dimension dim = Matrix::get_null_value<Dimension>());
/**
* @brief Only available for @ref mp_matrices "non-basic matrices".
* It does the same as the other version, but allows the boundary cells to be identified without restrictions
Expand All @@ -787,7 +795,8 @@ class Matrix {
* chains used to reduce the boundary. Otherwise, nothing.
*/
template <class Boundary_range = Boundary>
Insertion_return insert_boundary(ID_index cellIndex, const Boundary_range& boundary, Dimension dim = -1);
Insertion_return insert_boundary(ID_index cellIndex, const Boundary_range& boundary,
Dimension dim = Matrix::get_null_value<Dimension>());

/**
* @brief Returns the column at the given @ref MatIdx index.
Expand Down Expand Up @@ -1510,7 +1519,7 @@ template <class PersistenceMatrixOptions>
inline void Matrix<PersistenceMatrixOptions>::set_characteristic(Characteristic characteristic)
{
if constexpr (!PersistenceMatrixOptions::is_z2) {
if (colSettings_->operators.get_characteristic() != static_cast<Characteristic>(-1)) {
if (colSettings_->operators.get_characteristic() != get_null_value<Characteristic>()) {
std::cerr << "Warning: Characteristic already initialised. Changing it could lead to incoherences in the matrix "
"as the modulo was already applied to values in existing columns.";
}
Expand All @@ -1524,7 +1533,7 @@ template <class Container>
inline void Matrix<PersistenceMatrixOptions>::insert_column(const Container& column)
{
if constexpr (!PersistenceMatrixOptions::is_z2){
GUDHI_CHECK(colSettings_->operators.get_characteristic() != static_cast<Characteristic>(-1),
GUDHI_CHECK(colSettings_->operators.get_characteristic() != get_null_value<Characteristic>(),
std::logic_error("Matrix::insert_column - Columns cannot be initialized if the coefficient field "
"characteristic is not specified."));
}
Expand All @@ -1540,7 +1549,7 @@ template <class Container>
inline void Matrix<PersistenceMatrixOptions>::insert_column(const Container& column, Index columnIndex)
{
if constexpr (!PersistenceMatrixOptions::is_z2){
GUDHI_CHECK(colSettings_->operators.get_characteristic() != static_cast<Characteristic>(-1),
GUDHI_CHECK(colSettings_->operators.get_characteristic() != get_null_value<Characteristic>(),
std::logic_error("Matrix::insert_column - Columns cannot be initialized if the coefficient field "
"characteristic is not specified."));
}
Expand All @@ -1558,7 +1567,7 @@ inline typename Matrix<PersistenceMatrixOptions>::Insertion_return
Matrix<PersistenceMatrixOptions>::insert_boundary(const Boundary_range& boundary, Dimension dim)
{
if constexpr (!PersistenceMatrixOptions::is_z2){
GUDHI_CHECK(colSettings_->operators.get_characteristic() != static_cast<Characteristic>(-1),
GUDHI_CHECK(colSettings_->operators.get_characteristic() != get_null_value<Characteristic>(),
std::logic_error("Matrix::insert_boundary - Columns cannot be initialized if the coefficient field "
"characteristic is not specified."));
}
Expand All @@ -1578,7 +1587,7 @@ Matrix<PersistenceMatrixOptions>::insert_boundary(ID_index cellIndex,
Dimension dim)
{
if constexpr (!PersistenceMatrixOptions::is_z2){
GUDHI_CHECK(colSettings_->operators.get_characteristic() != static_cast<Characteristic>(-1),
GUDHI_CHECK(colSettings_->operators.get_characteristic() != get_null_value<Characteristic>(),
std::logic_error("Matrix::insert_boundary - Columns cannot be initialized if the coefficient field "
"characteristic is not specified."));
}
Expand Down
5 changes: 3 additions & 2 deletions src/Persistence_matrix/include/gudhi/Persistence_matrix/Base_matrix.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ class Base_matrix : public Master_matrix::template Base_swap_option<Base_matrix<
* @param dim Ignored.
*/
template <class Boundary_range>
void insert_boundary(const Boundary_range& boundary, Dimension dim = -1);
void insert_boundary(const Boundary_range& boundary,
Dimension dim = Master_matrix::template get_null_value<Dimension>());
/**
* @brief Returns the column at the given @ref MatIdx index.
* The type of the column depends on the choosen options, see @ref PersistenceMatrixOptions::column_type.
Expand Down Expand Up @@ -438,7 +439,7 @@ template <class Master_matrix>
template <class Boundary_range>
inline void Base_matrix<Master_matrix>::insert_boundary(const Boundary_range& boundary, Dimension dim)
{
if (dim == -1) dim = boundary.size() == 0 ? 0 : boundary.size() - 1;
if (dim == Master_matrix::template get_null_value<Dimension>()) dim = boundary.size() == 0 ? 0 : boundary.size() - 1;
//TODO: dim not actually stored right now, so either get rid of it or store it again
_insert(boundary, nextInsertIndex_++, dim);
}
Expand Down
5 changes: 3 additions & 2 deletions ...Persistence_matrix/include/gudhi/Persistence_matrix/Base_matrix_with_column_compression.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ class Base_matrix_with_column_compression : protected Master_matrix::Matrix_row_
* @param dim Ignored.
*/
template <class Boundary_range>
void insert_boundary(const Boundary_range& boundary, Dimension dim = -1);
void insert_boundary(const Boundary_range& boundary,
Dimension dim = Master_matrix::template get_null_value<Dimension>());
/**
* @brief Returns the column at the given @ref MatIdx index.
* The type of the column depends on the choosen options, see @ref PersistenceMatrixOptions::column_type.
Expand Down Expand Up @@ -450,7 +451,7 @@ inline void Base_matrix_with_column_compression<Master_matrix>::insert_boundary(
// handles a dimension which is not actually stored.
// TODO: verify if this is not a problem for the cohomology algorithm and if yes,
// change how Column_dimension_option is choosen.
if (dim == -1) dim = boundary.size() == 0 ? 0 : boundary.size() - 1;
if (dim == Master_matrix::template get_null_value<Dimension>()) dim = boundary.size() == 0 ? 0 : boundary.size() - 1;

if constexpr (Master_matrix::Option_list::has_row_access && !Master_matrix::Option_list::has_removable_rows) {
if (boundary.begin() != boundary.end()) {
Expand Down
20 changes: 11 additions & 9 deletions src/Persistence_matrix/include/gudhi/Persistence_matrix/Boundary_matrix.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ class Boundary_matrix : public Master_matrix::Matrix_dimension_option,
* @return The @ref MatIdx index of the inserted boundary.
*/
template <class Boundary_range = Boundary>
Index insert_boundary(const Boundary_range& boundary, Dimension dim = -1);
Index insert_boundary(const Boundary_range& boundary,
Dimension dim = Master_matrix::template get_null_value<Dimension>());
/**
* @brief It does the same as the other version, but allows the boundary cells to be identified without restrictions
* except that all IDs have to be strictly increasing in the order of filtration. Note that you should avoid then
Expand All @@ -161,7 +162,8 @@ class Boundary_matrix : public Master_matrix::Matrix_dimension_option,
* @return The @ref MatIdx index of the inserted boundary.
*/
template <class Boundary_range = Boundary>
Index insert_boundary(ID_index cellIndex, const Boundary_range& boundary, Dimension dim = -1);
Index insert_boundary(ID_index cellIndex, const Boundary_range& boundary,
Dimension dim = Master_matrix::template get_null_value<Dimension>());
/**
* @brief Returns the column at the given @ref MatIdx index.
* The type of the column depends on the choosen options, see @ref PersistenceMatrixOptions::column_type.
Expand Down Expand Up @@ -384,7 +386,7 @@ class Boundary_matrix : public Master_matrix::Matrix_dimension_option,

template <class Master_matrix>
inline Boundary_matrix<Master_matrix>::Boundary_matrix(Column_settings* colSettings)
: Dim_opt(-1),
: Dim_opt(Master_matrix::template get_null_value<Dimension>()),
Swap_opt(),
Pair_opt(),
RA_opt(),
Expand All @@ -396,7 +398,7 @@ template <class Master_matrix>
template <class Boundary_range>
inline Boundary_matrix<Master_matrix>::Boundary_matrix(const std::vector<Boundary_range>& orderedBoundaries,
Column_settings* colSettings)
: Dim_opt(-1),
: Dim_opt(Master_matrix::template get_null_value<Dimension>()),
Swap_opt(orderedBoundaries.size()),
Pair_opt(),
RA_opt(orderedBoundaries.size()),
Expand All @@ -413,7 +415,7 @@ inline Boundary_matrix<Master_matrix>::Boundary_matrix(const std::vector<Boundar
template <class Master_matrix>
inline Boundary_matrix<Master_matrix>::Boundary_matrix(unsigned int numberOfColumns,
Column_settings* colSettings)
: Dim_opt(-1),
: Dim_opt(Master_matrix::template get_null_value<Dimension>()),
Swap_opt(numberOfColumns),
Pair_opt(),
RA_opt(numberOfColumns),
Expand Down Expand Up @@ -471,7 +473,7 @@ template <class Boundary_range>
inline typename Boundary_matrix<Master_matrix>::Index Boundary_matrix<Master_matrix>::insert_boundary(
ID_index cellIndex, const Boundary_range& boundary, Dimension dim)
{
if (dim == -1) dim = boundary.size() == 0 ? 0 : boundary.size() - 1;
if (dim == Master_matrix::template get_null_value<Dimension>()) dim = boundary.size() == 0 ? 0 : boundary.size() - 1;

_orderRowsIfNecessary();

Expand Down Expand Up @@ -543,7 +545,7 @@ inline typename Boundary_matrix<Master_matrix>::Index Boundary_matrix<Master_mat
static_assert(Master_matrix::Option_list::has_removable_columns,
"'remove_last' is not implemented for the chosen options.");

if (nextInsertIndex_ == 0) return -1; // empty matrix
if (nextInsertIndex_ == 0) return Master_matrix::template get_null_value<Index>(); // empty matrix
--nextInsertIndex_;

//updates dimension max
Expand All @@ -558,7 +560,7 @@ inline typename Boundary_matrix<Master_matrix>::Index Boundary_matrix<Master_mat
pivot = it->second.get_pivot();
if constexpr (activeSwapOption) {
// if the removed column is positive, the pivot won't change value
if (Swap_opt::rowSwapped_ && pivot != static_cast<ID_index>(-1)) {
if (Swap_opt::rowSwapped_ && pivot != Master_matrix::template get_null_value<ID_index>()) {
Swap_opt::_orderRows();
pivot = it->second.get_pivot();
}
Expand All @@ -568,7 +570,7 @@ inline typename Boundary_matrix<Master_matrix>::Index Boundary_matrix<Master_mat
pivot = matrix_[nextInsertIndex_].get_pivot();
if constexpr (activeSwapOption) {
// if the removed column is positive, the pivot won't change value
if (Swap_opt::rowSwapped_ && pivot != static_cast<ID_index>(-1)) {
if (Swap_opt::rowSwapped_ && pivot != Master_matrix::template get_null_value<ID_index>()) {
Swap_opt::_orderRows();
pivot = matrix_[nextInsertIndex_].get_pivot();
}
Expand Down
Loading

0 comments on commit bf241cd

Please sign in to comment.