Skip to content

Commit

Permalink
replace static_cast(-1) by get_null_value in persistence matrix
Browse files Browse the repository at this point in the history
  • Loading branch information
hschreiber committed Nov 7, 2024
1 parent 29f4312 commit 98ff660
Show file tree
Hide file tree
Showing 21 changed files with 111 additions and 86 deletions.
27 changes: 18 additions & 9 deletions src/Persistence_matrix/include/gudhi/Matrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,16 @@ class Matrix {
*/
using Element = typename Field_operators::Element;
using Characteristic = typename Field_operators::Characteristic;


/**
* @private
* @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 +376,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 +601,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 = 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 +680,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 = get_null_value<Characteristic>());
/**
* @brief Copy constructor.
*
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
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,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 +568,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
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,9 @@ inline std::vector<typename Master_matrix::Entry_representative> Chain_matrix<Ma
}

if constexpr (Master_matrix::Option_list::has_matrix_maximal_dimension_access) {
Dim_opt::update_up(dim == static_cast<Dimension>(-1) ? (boundary.size() == 0 ? 0 : boundary.size() - 1) : dim);
Dim_opt::update_up(dim == Master_matrix::template get_null_value<Dimension>()
? (boundary.size() == 0 ? 0 : boundary.size() - 1)
: dim);
}

return _reduce_boundary(cellID, boundary, dim);
Expand Down Expand Up @@ -915,7 +917,7 @@ inline void Chain_matrix<Master_matrix>::print() const
if constexpr (!Master_matrix::Option_list::has_map_column_container) {
for (ID_index i = 0; i < pivotToColumnIndex_.size(); ++i) {
Index pos = pivotToColumnIndex_[i];
if (pos != static_cast<Index>(-1)){
if (pos != Master_matrix::template get_null_value<Index>()){
const Column& col = matrix_[pos];
for (const auto& entry : col) {
std::cout << entry.get_row_index() << " ";
Expand All @@ -928,7 +930,7 @@ inline void Chain_matrix<Master_matrix>::print() const
std::cout << "Row Matrix:\n";
for (ID_index i = 0; i < pivotToColumnIndex_.size(); ++i) {
Index pos = pivotToColumnIndex_[i];
if (pos != static_cast<Index>(-1)){
if (pos != Master_matrix::template get_null_value<Index>()){
const Row& row = RA_opt::get_row(pos);
for (const auto& entry : row) {
std::cout << entry.get_column_index() << " ";
Expand Down Expand Up @@ -966,7 +968,8 @@ inline std::vector<typename Master_matrix::Entry_representative> Chain_matrix<Ma
ID_index cellID, const Boundary_range& boundary, Dimension dim)
{
Tmp_column column(boundary.begin(), boundary.end());
if (dim == static_cast<Dimension>(-1)) dim = boundary.begin() == boundary.end() ? 0 : boundary.size() - 1;
if (dim == Master_matrix::template get_null_value<Dimension>())
dim = boundary.begin() == boundary.end() ? 0 : boundary.size() - 1;
std::vector<Entry_representative> chainsInH; // for corresponding indices in H (paired columns)
std::vector<Entry_representative> chainsInF; // for corresponding indices in F (unpaired, essential columns)

Expand Down Expand Up @@ -1225,7 +1228,7 @@ inline void Chain_matrix<Master_matrix>::_remove_last(Index lastIndex)
auto it = _indexToBar().find(--_nextPosition());
typename Barcode::iterator bar = it->second;

if (bar->death == static_cast<Pos_index>(-1))
if (bar->death == Master_matrix::template get_null_value<Pos_index>())
_barcode().erase(bar);
else
bar->death = -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -760,8 +760,9 @@ inline void Id_to_index_overlay<Underlying_matrix, Master_matrix>::insert_bounda
GUDHI_CHECK(idToIndex_->find(cellIndex) == idToIndex_->end(),
std::invalid_argument("Id_to_index_overlay::insert_boundary - Index for simplex already chosen!"));
} else {
GUDHI_CHECK((idToIndex_->size() <= cellIndex || _id_to_index(cellIndex) == static_cast<Index>(-1)),
std::invalid_argument("Id_to_index_overlay::insert_boundary - Index for simplex already chosen!"));
GUDHI_CHECK(
(idToIndex_->size() <= cellIndex || _id_to_index(cellIndex) == Master_matrix::template get_null_value<Index>()),
std::invalid_argument("Id_to_index_overlay::insert_boundary - Index for simplex already chosen!"));
}
matrix_.insert_boundary(cellIndex, boundary, dim);
if constexpr (Master_matrix::Option_list::is_of_boundary_type) {
Expand Down Expand Up @@ -808,7 +809,7 @@ inline void Id_to_index_overlay<Underlying_matrix, Master_matrix>::remove_maxima
}
} else {
for (ID_index i = 0; i < idToIndex_->size(); ++i) {
if (_id_to_index(i) != static_cast<Index>(-1)) indexToID[_id_to_index(i)] = i;
if (_id_to_index(i) != Master_matrix::template get_null_value<Index>()) indexToID[_id_to_index(i)] = i;
}
}
--nextIndex_;
Expand Down Expand Up @@ -857,7 +858,8 @@ inline void Id_to_index_overlay<Underlying_matrix, Master_matrix>::remove_last()
idToIndex_->erase(it);
} else {
Index id = idToIndex_->size() - 1;
while (_id_to_index(id) == static_cast<Index>(-1)) --id; // should always stop before reaching -1
// should always stop before reaching -1
while (_id_to_index(id) == Master_matrix::template get_null_value<Index>()) --id;
GUDHI_CHECK(_id_to_index(id) == nextIndex_,
std::logic_error("Id_to_index_overlay::remove_last - Indexation problem."));
_id_to_index(id) = -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ inline void RU_matrix<Master_matrix>::remove_last()
pivotToColumnIndex_.erase(reducedMatrixR_.remove_last());
} else {
ID_index lastPivot = reducedMatrixR_.remove_last();
if (lastPivot != static_cast<ID_index>(-1)) pivotToColumnIndex_[lastPivot] = -1;
if (lastPivot != Master_matrix::template get_null_value<ID_index>()) pivotToColumnIndex_[lastPivot] = -1;
}

// if has_vine_update and has_column_pairings are both true,
Expand Down Expand Up @@ -752,7 +752,7 @@ inline void RU_matrix<Master_matrix>::_insert_boundary(Index currentIndex)

if constexpr (!Master_matrix::Option_list::has_map_column_container) {
ID_index pivot = reducedMatrixR_.get_column(currentIndex).get_pivot();
if (pivot != static_cast<ID_index>(-1) && pivotToColumnIndex_.size() <= pivot)
if (pivot != Master_matrix::template get_null_value<ID_index>() && pivotToColumnIndex_.size() <= pivot)
pivotToColumnIndex_.resize((pivot + 1) * 2, -1);
}

Expand Down Expand Up @@ -806,7 +806,7 @@ template <class Master_matrix>
inline void RU_matrix<Master_matrix>::_reduce_column(Index target, Index eventIndex)
{
auto get_column_with_pivot_ = [&](ID_index pivot) -> Index {
if (pivot == static_cast<ID_index>(-1)) return -1;
if (pivot == Master_matrix::template get_null_value<ID_index>()) return -1;
if constexpr (Master_matrix::Option_list::has_map_column_container) {
auto it = pivotToColumnIndex_.find(pivot);
if (it == pivotToColumnIndex_.end())
Expand All @@ -822,13 +822,14 @@ inline void RU_matrix<Master_matrix>::_reduce_column(Index target, Index eventIn
ID_index pivot = curr.get_pivot();
Index currIndex = get_column_with_pivot_(pivot);

while (pivot != static_cast<ID_index>(-1) && currIndex != static_cast<Index>(-1)) {
while (pivot != Master_matrix::template get_null_value<ID_index>() &&
currIndex != Master_matrix::template get_null_value<Index>()) {
_reduce_column_by(target, currIndex);
pivot = curr.get_pivot();
currIndex = get_column_with_pivot_(pivot);
}

if (pivot != static_cast<ID_index>(-1)) {
if (pivot != Master_matrix::template get_null_value<ID_index>()) {
if constexpr (Master_matrix::Option_list::has_map_column_container) {
pivotToColumnIndex_.try_emplace(pivot, target);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ inline void Base_pairing<Master_matrix>::_reduce()
auto itNeg = negativeColumns.find(pivotColumnNumber);
Index pivotKiller = itNeg == negativeColumns.end() ? -1 : itNeg->second;

while (pivot != static_cast<ID_index>(-1) && pivotKiller != static_cast<Index>(-1)) {
while (pivot != Master_matrix::template get_null_value<ID_index>() &&
pivotKiller != Master_matrix::template get_null_value<Index>()) {
if constexpr (Master_matrix::Option_list::is_z2) {
curr += _matrix()->get_column(pivotKiller);
} else {
Expand All @@ -174,7 +175,7 @@ inline void Base_pairing<Master_matrix>::_reduce()
pivotKiller = itNeg == negativeColumns.end() ? -1 : itNeg->second;
}

if (pivot != static_cast<ID_index>(-1)) {
if (pivot != Master_matrix::template get_null_value<ID_index>()) {
negativeColumns.emplace(pivotColumnNumber, i);
_matrix()->get_column(pivotColumnNumber).clear();
barcode_.emplace_back(pivotColumnNumber, i, dim - 1);
Expand All @@ -193,7 +194,7 @@ inline void Base_pairing<Master_matrix>::_reduce()
// map can only be constructed once barcode is sorted
for (Index i = 0; i < barcode_.size(); ++i) {
auto d = barcode_[i].death;
if (d != static_cast<Pos_index>(-1)) {
if (d != Master_matrix::template get_null_value<Pos_index>()) {
deathToBar_.emplace(d, i);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class Chain_column_extra_properties
* @return true If the column is paired.
* @return false Otherwise.
*/
bool is_paired() const { return pairedColumn_ != static_cast<Index>(-1); }
bool is_paired() const { return pairedColumn_ != Master_matrix::template get_null_value<Index>(); }
/**
* @brief Sets the value of the pair.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ inline typename Heap_column<Master_matrix>::Field_element Heap_column<Master_mat
return 0;
} else {
Field_element sum(0);
if (Chain_opt::get_pivot() == static_cast<ID_index>(-1)) return sum;
if (Chain_opt::get_pivot() == Master_matrix::template get_null_value<ID_index>()) return sum;
for (const Entry* entry : column_) {
if (entry->get_row_index() == Chain_opt::get_pivot()) operators_->add_inplace(sum, entry->get_element());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ inline void Intrusive_list_column<Master_matrix>::reorder(const Row_index_map& v
Entry* entry = &(*it);
if constexpr (Master_matrix::Option_list::has_row_access) {
RA_opt::unlink(entry);
if (columnIndex != static_cast<Index>(-1)) entry->set_column_index(columnIndex);
if (columnIndex != Master_matrix::template get_null_value<Index>()) entry->set_column_index(columnIndex);
}
entry->set_row_index(valueMap.at(entry->get_row_index()));
if constexpr (Master_matrix::Option_list::has_intrusive_rows && Master_matrix::Option_list::has_row_access)
Expand Down Expand Up @@ -576,7 +576,7 @@ Intrusive_list_column<Master_matrix>::get_pivot_value() const
if (column_.empty()) return 0;
return column_.back().get_element();
} else {
if (Chain_opt::get_pivot() == static_cast<ID_index>(-1)) return Field_element();
if (Chain_opt::get_pivot() == Master_matrix::template get_null_value<ID_index>()) return Field_element();
for (const Entry& entry : column_) {
if (entry.get_row_index() == Chain_opt::get_pivot()) return entry.get_element();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,8 @@ inline void Intrusive_set_column<Master_matrix>::reorder(const Row_index_map& va
if constexpr (Master_matrix::Option_list::has_row_access) {
for (auto it = column_.begin(); it != column_.end();) {
Entry* newEntry = entryPool_->construct(
columnIndex == static_cast<Index>(-1) ? RA_opt::columnIndex_ : columnIndex, valueMap.at(it->get_row_index()));
columnIndex == Master_matrix::template get_null_value<Index>() ? RA_opt::columnIndex_ : columnIndex,
valueMap.at(it->get_row_index()));
if constexpr (!Master_matrix::Option_list::is_z2) {
newEntry->set_element(it->get_element());
}
Expand Down Expand Up @@ -578,7 +579,7 @@ Intrusive_set_column<Master_matrix>::get_pivot_value() const
if (column_.empty()) return 0;
return column_.rbegin()->get_element();
} else {
if (Chain_opt::get_pivot() == static_cast<ID_index>(-1)) return 0;
if (Chain_opt::get_pivot() == Master_matrix::template get_null_value<ID_index>()) return 0;
auto it = column_.find(Entry(Chain_opt::get_pivot()));
GUDHI_CHECK(it != column_.end(),
"Intrusive_set_column::get_pivot_value - Pivot not found only if the column was misused.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ inline void List_column<Master_matrix>::reorder(const Row_index_map& valueMap, [
Entry* entry = *it;
if constexpr (Master_matrix::Option_list::has_row_access) {
RA_opt::unlink(entry);
if (columnIndex != static_cast<Index>(-1)) entry->set_column_index(columnIndex);
if (columnIndex != Master_matrix::template get_null_value<Index>()) entry->set_column_index(columnIndex);
}
entry->set_row_index(valueMap.at(entry->get_row_index()));
if constexpr (Master_matrix::Option_list::has_intrusive_rows && Master_matrix::Option_list::has_row_access)
Expand Down Expand Up @@ -569,7 +569,7 @@ inline typename List_column<Master_matrix>::Field_element List_column<Master_mat
if (column_.empty()) return 0;
return column_.back()->get_element();
} else {
if (Chain_opt::get_pivot() == static_cast<ID_index>(-1)) return Field_element();
if (Chain_opt::get_pivot() == Master_matrix::template get_null_value<ID_index>()) return Field_element();
for (const Entry* entry : column_) {
if (entry->get_row_index() == Chain_opt::get_pivot()) return entry->get_element();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ inline void Naive_vector_column<Master_matrix>::reorder(const Row_index_map& val
for (Entry* entry : column_) {
if constexpr (Master_matrix::Option_list::has_row_access) {
RA_opt::unlink(entry);
if (columnIndex != static_cast<Index>(-1)) entry->set_column_index(columnIndex);
if (columnIndex != Master_matrix::template get_null_value<Index>()) entry->set_column_index(columnIndex);
}
entry->set_row_index(valueMap.at(entry->get_row_index()));
if constexpr (Master_matrix::Option_list::has_intrusive_rows && Master_matrix::Option_list::has_row_access)
Expand Down Expand Up @@ -559,7 +559,7 @@ inline typename Naive_vector_column<Master_matrix>::Field_element Naive_vector_c
if constexpr (Master_matrix::Option_list::is_of_boundary_type) {
return column_.empty() ? Field_element() : column_.back()->get_element();
} else {
if (Chain_opt::get_pivot() == static_cast<ID_index>(-1)) return Field_element();
if (Chain_opt::get_pivot() == Master_matrix::template get_null_value<ID_index>()) return Field_element();
for (const Entry* entry : column_) {
if (entry->get_row_index() == Chain_opt::get_pivot()) return entry->get_element();
}
Expand Down
Loading

0 comments on commit 98ff660

Please sign in to comment.