Skip to content

Commit

Permalink
replacing remaining -1s in the matrix module
Browse files Browse the repository at this point in the history
  • Loading branch information
hschreiber committed Nov 12, 2024
1 parent fe6b11d commit c7ae079
Show file tree
Hide file tree
Showing 25 changed files with 420 additions and 235 deletions.
16 changes: 9 additions & 7 deletions src/Persistence_matrix/concept/PersistenceMatrixColumn.h
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
6 changes: 3 additions & 3 deletions src/Persistence_matrix/include/gudhi/Matrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ class Matrix {
using Characteristic = typename Field_operators::Characteristic;

/**
* @private
* @brief Returns value from a type when not set.
*/
template <typename T>
Expand Down Expand Up @@ -774,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 @@ -796,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
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
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
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 Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ class Chain_matrix : public Master_matrix::Matrix_dimension_option,
* @return The @ref MatIdx indices of the unpaired chains used to reduce the boundary.
*/
template <class Boundary_range = Boundary>
std::vector<Entry_representative> insert_boundary(const Boundary_range& boundary, Dimension dim = -1);
std::vector<Entry_representative> 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 @@ -267,9 +268,10 @@ class Chain_matrix : public Master_matrix::Matrix_dimension_option,
* @return The @ref MatIdx index of the inserted boundary.
*/
template <class Boundary_range = Boundary>
std::vector<Entry_representative> insert_boundary(ID_index cellID,
const Boundary_range& boundary,
Dimension dim = -1);
std::vector<Entry_representative> insert_boundary(
ID_index cellID,
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 @@ -523,7 +525,7 @@ class Chain_matrix : public Master_matrix::Matrix_dimension_option,

template <class Master_matrix>
inline Chain_matrix<Master_matrix>::Chain_matrix(Column_settings* colSettings)
: Dim_opt(-1),
: Dim_opt(Master_matrix::template get_null_value<Dimension>()),
Pair_opt(),
Swap_opt(),
Rep_opt(),
Expand All @@ -536,7 +538,7 @@ template <class Master_matrix>
template <class Boundary_range>
inline Chain_matrix<Master_matrix>::Chain_matrix(const std::vector<Boundary_range>& orderedBoundaries,
Column_settings* colSettings)
: Dim_opt(-1),
: Dim_opt(Master_matrix::template get_null_value<Dimension>()),
Pair_opt(),
Swap_opt(),
Rep_opt(),
Expand All @@ -548,7 +550,7 @@ inline Chain_matrix<Master_matrix>::Chain_matrix(const std::vector<Boundary_rang
if constexpr (Master_matrix::Option_list::has_map_column_container) {
pivotToColumnIndex_.reserve(orderedBoundaries.size());
} else {
pivotToColumnIndex_.resize(orderedBoundaries.size(), -1);
pivotToColumnIndex_.resize(orderedBoundaries.size(), Master_matrix::template get_null_value<Index>());
}

for (const Boundary_range& b : orderedBoundaries) {
Expand All @@ -559,7 +561,7 @@ inline Chain_matrix<Master_matrix>::Chain_matrix(const std::vector<Boundary_rang
template <class Master_matrix>
inline Chain_matrix<Master_matrix>::Chain_matrix(unsigned int numberOfColumns,
Column_settings* colSettings)
: Dim_opt(-1),
: Dim_opt(Master_matrix::template get_null_value<Dimension>()),
Pair_opt(),
Swap_opt(),
Rep_opt(),
Expand All @@ -571,7 +573,7 @@ inline Chain_matrix<Master_matrix>::Chain_matrix(unsigned int numberOfColumns,
if constexpr (Master_matrix::Option_list::has_map_column_container) {
pivotToColumnIndex_.reserve(numberOfColumns);
} else {
pivotToColumnIndex_.resize(numberOfColumns, -1);
pivotToColumnIndex_.resize(numberOfColumns, Master_matrix::template get_null_value<Index>());
}
}

Expand All @@ -580,7 +582,7 @@ template <typename BirthComparatorFunction, typename DeathComparatorFunction>
inline Chain_matrix<Master_matrix>::Chain_matrix(Column_settings* colSettings,
const BirthComparatorFunction& birthComparator,
const DeathComparatorFunction& deathComparator)
: Dim_opt(-1),
: Dim_opt(Master_matrix::template get_null_value<Dimension>()),
Pair_opt(),
Swap_opt(birthComparator, deathComparator),
Rep_opt(),
Expand All @@ -595,7 +597,7 @@ inline Chain_matrix<Master_matrix>::Chain_matrix(const std::vector<Boundary_rang
Column_settings* colSettings,
const BirthComparatorFunction& birthComparator,
const DeathComparatorFunction& deathComparator)
: Dim_opt(-1),
: Dim_opt(Master_matrix::template get_null_value<Dimension>()),
Pair_opt(),
Swap_opt(birthComparator, deathComparator),
Rep_opt(),
Expand All @@ -607,7 +609,7 @@ inline Chain_matrix<Master_matrix>::Chain_matrix(const std::vector<Boundary_rang
if constexpr (Master_matrix::Option_list::has_map_column_container) {
pivotToColumnIndex_.reserve(orderedBoundaries.size());
} else {
pivotToColumnIndex_.resize(orderedBoundaries.size(), -1);
pivotToColumnIndex_.resize(orderedBoundaries.size(), Master_matrix::template get_null_value<Index>());
}
for (const Boundary_range& b : orderedBoundaries) {
insert_boundary(b);
Expand All @@ -620,7 +622,7 @@ inline Chain_matrix<Master_matrix>::Chain_matrix(unsigned int numberOfColumns,
Column_settings* colSettings,
const BirthComparatorFunction& birthComparator,
const DeathComparatorFunction& deathComparator)
: Dim_opt(-1),
: Dim_opt(Master_matrix::template get_null_value<Dimension>()),
Pair_opt(),
Swap_opt(birthComparator, deathComparator),
Rep_opt(),
Expand All @@ -632,7 +634,7 @@ inline Chain_matrix<Master_matrix>::Chain_matrix(unsigned int numberOfColumns,
if constexpr (Master_matrix::Option_list::has_map_column_container) {
pivotToColumnIndex_.reserve(numberOfColumns);
} else {
pivotToColumnIndex_.resize(numberOfColumns, -1);
pivotToColumnIndex_.resize(numberOfColumns, Master_matrix::template get_null_value<Index>());
}
}

Expand Down Expand Up @@ -685,7 +687,7 @@ inline std::vector<typename Master_matrix::Entry_representative> Chain_matrix<Ma
{
if constexpr (!Master_matrix::Option_list::has_map_column_container) {
if (pivotToColumnIndex_.size() <= cellID) {
pivotToColumnIndex_.resize(cellID * 2 + 1, -1);
pivotToColumnIndex_.resize(cellID * 2 + 1, Master_matrix::template get_null_value<Index>());
}
}

Expand All @@ -694,7 +696,8 @@ inline std::vector<typename Master_matrix::Entry_representative> Chain_matrix<Ma
Swap_opt::CP::pivotToPosition_.try_emplace(cellID, _nextPosition());
} else {
if (Swap_opt::CP::pivotToPosition_.size() <= cellID)
Swap_opt::CP::pivotToPosition_.resize(pivotToColumnIndex_.size(), -1);
Swap_opt::CP::pivotToPosition_.resize(pivotToColumnIndex_.size(),
Master_matrix::template get_null_value<Pos_index>());
Swap_opt::CP::pivotToPosition_[cellID] = _nextPosition();
}
}
Expand Down Expand Up @@ -1215,7 +1218,7 @@ inline void Chain_matrix<Master_matrix>::_remove_last(Index lastIndex)
}

if (colToErase.is_paired()) matrix_.at(colToErase.get_paired_chain_index()).unassign_paired_chain();
pivotToColumnIndex_[pivot] = -1;
pivotToColumnIndex_[pivot] = Master_matrix::template get_null_value<Index>();
matrix_.pop_back();
// TODO: resize matrix_ when a lot is removed? Could be not the best strategy if user inserts a lot back afterwards.
}
Expand All @@ -1231,7 +1234,7 @@ inline void Chain_matrix<Master_matrix>::_remove_last(Index lastIndex)
if (bar->death == Master_matrix::template get_null_value<Pos_index>())
_barcode().erase(bar);
else
bar->death = -1;
bar->death = Master_matrix::template get_null_value<Pos_index>();

_indexToBar().erase(it);
if constexpr (Master_matrix::Option_list::has_vine_update) Swap_opt::CP::pivotToPosition_.erase(pivot);
Expand Down Expand Up @@ -1268,7 +1271,7 @@ template <class Master_matrix>
inline void Chain_matrix<Master_matrix>::_add_bar(Dimension dim)
{
if constexpr (Master_matrix::Option_list::has_column_pairings) {
_barcode().emplace_back(_nextPosition(), -1, dim);
_barcode().emplace_back(_nextPosition(), Master_matrix::template get_null_value<Pos_index>(), dim);
if constexpr (Master_matrix::Option_list::has_removable_columns) {
_indexToBar().try_emplace(_nextPosition(), --_barcode().end());
} else {
Expand Down
Loading

0 comments on commit c7ae079

Please sign in to comment.