From 0c5a3c03ec7d13447faa5b8f4731246b949ee361 Mon Sep 17 00:00:00 2001 From: hschreiber Date: Fri, 4 Oct 2024 17:31:31 +0200 Subject: [PATCH 1/3] optimization for column reduction at construction for RU --- .../concept/PersistenceMatrixColumn.h | 9 ++ .../gudhi/Persistence_matrix/Base_matrix.h | 2 +- .../Persistence_matrix/Boundary_matrix.h | 4 +- .../gudhi/Persistence_matrix/RU_matrix.h | 14 +-- .../Persistence_matrix/columns/heap_column.h | 15 ++++ .../columns/intrusive_list_column.h | 14 +++ .../columns/intrusive_set_column.h | 14 +++ .../Persistence_matrix/columns/list_column.h | 14 +++ .../columns/naive_vector_column.h | 14 +++ .../Persistence_matrix/columns/set_column.h | 14 +++ .../columns/unordered_set_column.h | 14 +++ .../columns/vector_column.h | 14 +++ .../gudhi/Persistence_matrix/ru_rep_cycles.h | 2 +- src/Persistence_matrix/test/pm_matrix_tests.h | 88 ++++++------------- 14 files changed, 160 insertions(+), 72 deletions(-) diff --git a/src/Persistence_matrix/concept/PersistenceMatrixColumn.h b/src/Persistence_matrix/concept/PersistenceMatrixColumn.h index 8ae47e292..0be4d79f5 100644 --- a/src/Persistence_matrix/concept/PersistenceMatrixColumn.h +++ b/src/Persistence_matrix/concept/PersistenceMatrixColumn.h @@ -426,6 +426,15 @@ class PersistenceMatrixColumn : template PersistenceMatrixColumn& multiply_source_and_add(const Entry_range& column, const Field_element& val); + /** + * @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". + * + * @param entry Entry to push back. + */ + void push_back(const Entry& entry); + /** * @brief Equality comparator. Equal in the sense that what is "supposed" to be contained in the columns is equal, * not what is actually stored in the underlying container. For example, the underlying container of diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/Base_matrix.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/Base_matrix.h index c78cf4110..0630aabc0 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/Base_matrix.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/Base_matrix.h @@ -615,7 +615,7 @@ inline void Base_matrix::print() if constexpr (Master_matrix::Option_list::has_row_access) { std::cout << "Row Matrix:\n"; for (Index i = 0; i < nextInsertIndex_; ++i) { - const auto& row = RA_opt::rows_[i]; + const auto& row = (*RA_opt::rows_)[i]; for (const auto& entry : row) { std::cout << entry.get_column_index() << " "; } diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/Boundary_matrix.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/Boundary_matrix.h index 93de613c0..4ffb3fbe6 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/Boundary_matrix.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/Boundary_matrix.h @@ -729,8 +729,8 @@ inline void Boundary_matrix::print() if constexpr (Master_matrix::Option_list::has_row_access) { std::cout << "Row Matrix:\n"; for (ID_index i = 0; i < nextInsertIndex_; ++i) { - const auto& row = RA_opt::rows_[i]; - for (const auto& entry : row) { + const auto& row = (*RA_opt::rows_)[i]; + for (const typename Column::Entry& entry : row) { std::cout << entry.get_column_index() << " "; } std::cout << "(" << i << ")\n"; diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/RU_matrix.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/RU_matrix.h index 7c3661a8a..38ceae6a2 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/RU_matrix.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/RU_matrix.h @@ -638,7 +638,7 @@ inline void RU_matrix::add_to(Index sourceColumnIndex, Index targ { reducedMatrixR_.add_to(sourceColumnIndex, targetColumnIndex); // U transposed to avoid row operations - if constexpr (Master_matrix::Option_list::has_vine_update) + if constexpr (Master_matrix::Option_list::is_z2) mirrorMatrixU_.add_to(targetColumnIndex, sourceColumnIndex); else mirrorMatrixU_.add_to(sourceColumnIndex, targetColumnIndex); @@ -649,6 +649,8 @@ inline void RU_matrix::multiply_target_and_add_to(Index sourceCol const Field_element& coefficient, Index targetColumnIndex) { + static_assert(!Master_matrix::Option_list::is_z2, + "Multiplication with something else than the identity is not allowed with Z2 coefficients."); reducedMatrixR_.multiply_target_and_add_to(sourceColumnIndex, coefficient, targetColumnIndex); mirrorMatrixU_.multiply_target_and_add_to(sourceColumnIndex, coefficient, targetColumnIndex); } @@ -658,6 +660,8 @@ inline void RU_matrix::multiply_source_and_add_to(const Field_ele Index sourceColumnIndex, Index targetColumnIndex) { + static_assert(!Master_matrix::Option_list::is_z2, + "Multiplication with something else than the identity is not allowed with Z2 coefficients."); reducedMatrixR_.multiply_source_and_add_to(coefficient, sourceColumnIndex, targetColumnIndex); mirrorMatrixU_.multiply_source_and_add_to(coefficient, sourceColumnIndex, targetColumnIndex); } @@ -844,10 +848,7 @@ inline void RU_matrix::_reduce_column_by(Index target, Index sour curr += reducedMatrixR_.get_column(source); // to avoid having to do line operations during vineyards, U is transposed // TODO: explain this somewhere in the documentation... - if constexpr (Master_matrix::Option_list::has_vine_update) - mirrorMatrixU_.get_column(source) += mirrorMatrixU_.get_column(target); - else - mirrorMatrixU_.get_column(target) += mirrorMatrixU_.get_column(source); + mirrorMatrixU_.get_column(source).push_back(*mirrorMatrixU_.get_column(target).begin()); } else { Column& toadd = reducedMatrixR_.get_column(source); Field_element coef = toadd.get_pivot_value(); @@ -855,8 +856,9 @@ inline void RU_matrix::_reduce_column_by(Index target, Index sour operators_->multiply_inplace(coef, operators_->get_characteristic() - curr.get_pivot_value()); curr.multiply_source_and_add(toadd, coef); + // but no transposition for Zp, careful if there will be vineyard or rep cycles in Zp one day + // TODO: explain this somewhere in the documentation... mirrorMatrixU_.multiply_source_and_add_to(coef, source, target); - // mirrorMatrixU_.get_column(target).multiply_source_and_add(mirrorMatrixU_.get_column(source), coef); } } diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/heap_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/heap_column.h index 5a0e4dc5c..c7b0fa8e0 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/heap_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/heap_column.h @@ -134,6 +134,8 @@ class Heap_column : public Master_matrix::Column_dimension_option, public Master Heap_column& multiply_source_and_add(const Entry_range& column, const Field_element& val); Heap_column& multiply_source_and_add(Heap_column& column, const Field_element& val); + void push_back(const Entry& entry); + std::size_t compute_hash_value(); friend bool operator==(const Heap_column& c1, const Heap_column& c2) { @@ -868,6 +870,19 @@ inline Heap_column& Heap_column::multiply_source_a return *this; } +template +inline void Heap_column::push_back(const Entry& entry) +{ + static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + + Entry* newEntry = entryPool_->construct(entry.get_row_index()); + if constexpr (!Master_matrix::Option_list::is_z2) { + newEntry->set_element(operators_->get_value(entry.get_element())); + } + column_.push_back(newEntry); + std::push_heap(column_.begin(), column_.end(), entryPointerComp_); +} + template inline Heap_column& Heap_column::operator=(const Heap_column& other) { diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_list_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_list_column.h index d041852a8..773058019 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_list_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_list_column.h @@ -132,6 +132,8 @@ class Intrusive_list_column : public Master_matrix::Row_access_option, Intrusive_list_column& multiply_source_and_add(const Entry_range& column, const Field_element& val); Intrusive_list_column& multiply_source_and_add(Intrusive_list_column& column, const Field_element& val); + void push_back(const Entry& entry); + friend bool operator==(const Intrusive_list_column& c1, const Intrusive_list_column& c2) { if (&c1 == &c2) return true; @@ -821,6 +823,18 @@ inline Intrusive_list_column& Intrusive_list_column +inline void Intrusive_list_column::push_back(const Entry& entry) +{ + static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + + if constexpr (Master_matrix::Option_list::is_z2) { + _insert_entry(entry.get_row_index(), column_.end()); + } else { + _insert_entry(entry.get_element(), entry.get_row_index(), column_.end()); + } +} + template inline Intrusive_list_column& Intrusive_list_column::operator=( const Intrusive_list_column& other) diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_set_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_set_column.h index 3d1d00d61..17cb7d86b 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_set_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_set_column.h @@ -133,6 +133,8 @@ class Intrusive_set_column : public Master_matrix::Row_access_option, Intrusive_set_column& multiply_source_and_add(const Entry_range& column, const Field_element& val); Intrusive_set_column& multiply_source_and_add(Intrusive_set_column& column, const Field_element& val); + void push_back(const Entry& entry); + friend bool operator==(const Intrusive_set_column& c1, const Intrusive_set_column& c2) { if (&c1 == &c2) return true; @@ -821,6 +823,18 @@ inline Intrusive_set_column& Intrusive_set_column: return *this; } +template +inline void Intrusive_set_column::push_back(const Entry& entry) +{ + static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + + if constexpr (Master_matrix::Option_list::is_z2) { + _insert_entry(entry.get_row_index(), column_.end()); + } else { + _insert_entry(entry.get_element(), entry.get_row_index(), column_.end()); + } +} + template inline Intrusive_set_column& Intrusive_set_column::operator=( const Intrusive_set_column& other) diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/list_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/list_column.h index f21827a72..5793d5239 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/list_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/list_column.h @@ -131,6 +131,8 @@ class List_column : public Master_matrix::Row_access_option, List_column& multiply_source_and_add(const Entry_range& column, const Field_element& val); List_column& multiply_source_and_add(List_column& column, const Field_element& val); + void push_back(const Entry& entry); + friend bool operator==(const List_column& c1, const List_column& c2) { if (&c1 == &c2) return true; @@ -805,6 +807,18 @@ inline List_column& List_column::multiply_source_a return *this; } +template +inline void List_column::push_back(const Entry& entry) +{ + static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + + if constexpr (Master_matrix::Option_list::is_z2) { + _insert_entry(entry.get_row_index(), column_.end()); + } else { + _insert_entry(entry.get_element(), entry.get_row_index(), column_.end()); + } +} + template inline List_column& List_column::operator=(const List_column& other) { diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/naive_vector_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/naive_vector_column.h index e91484b31..21809f121 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/naive_vector_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/naive_vector_column.h @@ -131,6 +131,8 @@ class Naive_vector_column : public Master_matrix::Row_access_option, Naive_vector_column& multiply_source_and_add(const Entry_range& column, const Field_element& val); Naive_vector_column& multiply_source_and_add(Naive_vector_column& column, const Field_element& val); + void push_back(const Entry& entry); + friend bool operator==(const Naive_vector_column& c1, const Naive_vector_column& c2) { if (&c1 == &c2) return true; if (c1.column_.size() != c2.column_.size()) return false; @@ -801,6 +803,18 @@ inline Naive_vector_column& Naive_vector_column::m return *this; } +template +inline void Naive_vector_column::push_back(const Entry& entry) +{ + static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + + if constexpr (Master_matrix::Option_list::is_z2) { + _insert_entry(entry.get_row_index(), column_); + } else { + _insert_entry(entry.get_element(), entry.get_row_index(), column_); + } +} + template inline Naive_vector_column& Naive_vector_column::operator=( const Naive_vector_column& other) diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/set_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/set_column.h index de7151a75..c2efb18b2 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/set_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/set_column.h @@ -136,6 +136,8 @@ class Set_column : public Master_matrix::Row_access_option, Set_column& multiply_source_and_add(const Entry_range& column, const Field_element& val); Set_column& multiply_source_and_add(Set_column& column, const Field_element& val); + void push_back(const Entry& entry); + friend bool operator==(const Set_column& c1, const Set_column& c2) { if (&c1 == &c2) return true; @@ -797,6 +799,18 @@ inline Set_column& Set_column::multiply_source_and return *this; } +template +inline void Set_column::push_back(const Entry& entry) +{ + static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + + if constexpr (Master_matrix::Option_list::is_z2) { + _insert_entry(entry.get_row_index(), column_.end()); + } else { + _insert_entry(entry.get_element(), entry.get_row_index(), column_.end()); + } +} + template inline Set_column& Set_column::operator=(const Set_column& other) { diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/unordered_set_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/unordered_set_column.h index 4729b004d..9513a2022 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/unordered_set_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/unordered_set_column.h @@ -152,6 +152,8 @@ class Unordered_set_column : public Master_matrix::Row_access_option, Unordered_set_column& multiply_source_and_add(const Entry_range& column, const Field_element& val); Unordered_set_column& multiply_source_and_add(Unordered_set_column& column, const Field_element& val); + void push_back(const Entry& entry); + friend bool operator==(const Unordered_set_column& c1, const Unordered_set_column& c2) { if (&c1 == &c2) return true; if (c1.column_.size() != c2.column_.size()) return false; @@ -800,6 +802,18 @@ inline Unordered_set_column& Unordered_set_column: return *this; } +template +inline void Unordered_set_column::push_back(const Entry& entry) +{ + static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + + if constexpr (Master_matrix::Option_list::is_z2) { + _insert_entry(entry.get_row_index()); + } else { + _insert_entry(entry.get_element(), entry.get_row_index()); + } +} + template inline Unordered_set_column& Unordered_set_column::operator=( const Unordered_set_column& other) diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/vector_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/vector_column.h index 8f7d4237b..0721d86ed 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/vector_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/vector_column.h @@ -134,6 +134,8 @@ class Vector_column : public Master_matrix::Row_access_option, Vector_column& multiply_source_and_add(const Entry_range& column, const Field_element& val); Vector_column& multiply_source_and_add(Vector_column& column, const Field_element& val); + void push_back(const Entry& entry); + std::size_t compute_hash_value(); friend bool operator==(const Vector_column& c1, const Vector_column& c2) { @@ -903,6 +905,18 @@ inline Vector_column& Vector_column::multiply_sour return *this; } +template +inline void Vector_column::push_back(const Entry& entry) +{ + static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + + if constexpr (Master_matrix::Option_list::is_z2) { + _insert_entry(entry.get_row_index(), column_); + } else { + _insert_entry(entry.get_element(), entry.get_row_index(), column_); + } +} + template inline Vector_column& Vector_column::operator=(const Vector_column& other) { diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/ru_rep_cycles.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/ru_rep_cycles.h index ea77ccb9e..fb2e0c088 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/ru_rep_cycles.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/ru_rep_cycles.h @@ -135,7 +135,7 @@ inline RU_representative_cycles::RU_representative_cycles( template inline void RU_representative_cycles::update_representative_cycles() { - if constexpr (Master_matrix::Option_list::has_vine_update) { + if constexpr (Master_matrix::Option_list::is_z2) { birthToCycle_.clear(); birthToCycle_.resize(_matrix()->reducedMatrixR_.get_number_of_columns(), -1); Index c = 0; diff --git a/src/Persistence_matrix/test/pm_matrix_tests.h b/src/Persistence_matrix/test/pm_matrix_tests.h index dd2061265..3d0aac966 100644 --- a/src/Persistence_matrix/test/pm_matrix_tests.h +++ b/src/Persistence_matrix/test/pm_matrix_tests.h @@ -635,23 +635,13 @@ void test_ru_u_access() { std::vector > uColumns(7); if constexpr (Matrix::Option_list::is_z2) { - if constexpr (Matrix::Option_list::has_vine_update) { - uColumns[0] = {0}; - uColumns[1] = {1}; - uColumns[2] = {2}; - uColumns[3] = {3, 5}; - uColumns[4] = {4, 5}; - uColumns[5] = {5}; - uColumns[6] = {6}; - } else { - uColumns[0] = {0}; - uColumns[1] = {1}; - uColumns[2] = {2}; - uColumns[3] = {3}; - uColumns[4] = {4}; - uColumns[5] = {3, 4, 5}; - uColumns[6] = {6}; - } + uColumns[0] = {0}; + uColumns[1] = {1}; + uColumns[2] = {2}; + uColumns[3] = {3, 5}; + uColumns[4] = {4, 5}; + uColumns[5] = {5}; + uColumns[6] = {6}; } else { uColumns[0] = {{0, 1}}; uColumns[1] = {{1, 1}}; @@ -668,7 +658,7 @@ void test_ru_u_access() { test_column_equality(c, get_column_content_via_iterators(col)); } - if constexpr (Matrix::Option_list::has_vine_update) { + if constexpr (Matrix::Option_list::is_z2) { BOOST_CHECK(!m.is_zero_entry(3, 5, false)); BOOST_CHECK(!m.is_zero_column(4, false)); m.zero_entry(3, 5, false); @@ -775,23 +765,13 @@ void test_ru_u_row_access() { std::vector > rows; if constexpr (Matrix::Option_list::is_z2) { - if constexpr (Matrix::Option_list::has_vine_update) { - rows.push_back({0}); - rows.push_back({1}); - rows.push_back({2}); - rows.push_back({3}); - rows.push_back({4}); - rows.push_back({3, 4, 5}); - rows.push_back({6}); - } else { - rows.push_back({0}); - rows.push_back({1}); - rows.push_back({2}); - rows.push_back({3, 5}); - rows.push_back({4, 5}); - rows.push_back({5}); - rows.push_back({6}); - } + rows.push_back({0}); + rows.push_back({1}); + rows.push_back({2}); + rows.push_back({3}); + rows.push_back({4}); + rows.push_back({3, 4, 5}); + rows.push_back({6}); } else { rows.push_back({{0, 1}}); rows.push_back({{1, 1}}); @@ -1059,23 +1039,13 @@ void test_ru_operation() { std::vector > uColumns(7); if constexpr (Matrix::Option_list::is_z2) { - if constexpr (Matrix::Option_list::has_vine_update) { - uColumns[0] = {0}; - uColumns[1] = {1}; - uColumns[2] = {2}; - uColumns[3] = {3, 5}; - uColumns[4] = {4, 5}; - uColumns[5] = {5}; - uColumns[6] = {6}; - } else { - uColumns[0] = {0}; - uColumns[1] = {1}; - uColumns[2] = {2}; - uColumns[3] = {3}; - uColumns[4] = {4}; - uColumns[5] = {3, 4, 5}; - uColumns[6] = {6}; - } + uColumns[0] = {0}; + uColumns[1] = {1}; + uColumns[2] = {2}; + uColumns[3] = {3, 5}; + uColumns[4] = {4, 5}; + uColumns[5] = {5}; + uColumns[6] = {6}; } else { uColumns[0] = {{0, 1}}; uColumns[1] = {{1, 1}}; @@ -1097,10 +1067,7 @@ void test_ru_operation() { m.add_to(3, 5); if constexpr (Matrix::Option_list::is_z2) { columns[5] = {0, 1}; - if constexpr (Matrix::Option_list::has_vine_update) - uColumns[3] = {3}; - else - uColumns[5] = {4, 5}; + uColumns[3] = {3}; } else { columns[5] = {{0, 1}, {1, 4}}; uColumns[5] = {{4, 4}, {5, 1}}; @@ -1116,10 +1083,7 @@ void test_ru_operation() { m.add_to(4, 5); if constexpr (Matrix::Option_list::is_z2) { columns[5] = {0, 2}; - if constexpr (Matrix::Option_list::has_vine_update) - uColumns[4] = {4}; - else - uColumns[5] = {5}; + uColumns[4] = {4}; } else { columns[5] = {{0, 1}, {2, 4}}; uColumns[5] = {{5, 1}}; @@ -1132,11 +1096,11 @@ void test_ru_operation() { } } - if constexpr (!Matrix::Option_list::has_vine_update) { + if constexpr (!Matrix::Option_list::is_z2 || !is_RU()) { m.multiply_target_and_add_to(5, 3, 3); if constexpr (Matrix::Option_list::is_z2) { columns[3] = {1, 2}; - uColumns[3] = {3, 5}; + uColumns[5] = {3, 5}; } else { columns[3] = {{0, 4}, {1, 2}, {2, 4}}; uColumns[3] = {{3, 3}, {5, 1}}; From 26c7fb8aefc489e2f6bb925534b00f4be7ebb14a Mon Sep 17 00:00:00 2001 From: hschreiber Date: Mon, 14 Oct 2024 11:25:40 +0200 Subject: [PATCH 2/3] forgotten constexpr --- .../include/gudhi/Persistence_matrix/RU_matrix.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/RU_matrix.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/RU_matrix.h index 38ceae6a2..2020e0d11 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/RU_matrix.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/RU_matrix.h @@ -538,7 +538,7 @@ inline void RU_matrix::insert_boundary(ID_index cellIndex, if constexpr (Master_matrix::Option_list::has_vine_update) { if (cellIndex != nextEventIndex_) { Swap_opt::_positionToRowIdx().emplace(nextEventIndex_, cellIndex); - if (Master_matrix::Option_list::has_column_pairings) { + if constexpr (Master_matrix::Option_list::has_column_pairings) { Swap_opt::template RU_pairing::idToPosition_.emplace(cellIndex, nextEventIndex_); } } From e1b4634010a06b8595a6eb2721ca9dc491789044 Mon Sep 17 00:00:00 2001 From: hschreiber Date: Wed, 23 Oct 2024 11:15:47 +0200 Subject: [PATCH 3/3] add gudhi_check for column push_back --- .../include/gudhi/Persistence_matrix/columns/heap_column.h | 3 +++ .../gudhi/Persistence_matrix/columns/intrusive_list_column.h | 2 ++ .../gudhi/Persistence_matrix/columns/intrusive_set_column.h | 2 ++ .../include/gudhi/Persistence_matrix/columns/list_column.h | 2 ++ .../gudhi/Persistence_matrix/columns/naive_vector_column.h | 2 ++ .../include/gudhi/Persistence_matrix/columns/set_column.h | 2 ++ .../gudhi/Persistence_matrix/columns/unordered_set_column.h | 2 ++ .../include/gudhi/Persistence_matrix/columns/vector_column.h | 2 ++ 8 files changed, 17 insertions(+) diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/heap_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/heap_column.h index c7b0fa8e0..e195dfea4 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/heap_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/heap_column.h @@ -25,6 +25,7 @@ #include //std::swap, std::move & std::exchange #include +#include "gudhi/Debug_utils.h" #include @@ -875,6 +876,8 @@ inline void Heap_column::push_back(const Entry& entry) { static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + GUDHI_CHECK(entry.get_row_index() > get_pivot(), "The new row index has to be higher than the current pivot."); + Entry* newEntry = entryPool_->construct(entry.get_row_index()); if constexpr (!Master_matrix::Option_list::is_z2) { newEntry->set_element(operators_->get_value(entry.get_element())); diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_list_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_list_column.h index 773058019..0f5aaa832 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_list_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_list_column.h @@ -828,6 +828,8 @@ inline void Intrusive_list_column::push_back(const Entry& entry) { static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + GUDHI_CHECK(entry.get_row_index() > get_pivot(), "The new row index has to be higher than the current pivot."); + if constexpr (Master_matrix::Option_list::is_z2) { _insert_entry(entry.get_row_index(), column_.end()); } else { diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_set_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_set_column.h index 17cb7d86b..994a5014d 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_set_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/intrusive_set_column.h @@ -828,6 +828,8 @@ inline void Intrusive_set_column::push_back(const Entry& entry) { static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + GUDHI_CHECK(entry.get_row_index() > get_pivot(), "The new row index has to be higher than the current pivot."); + if constexpr (Master_matrix::Option_list::is_z2) { _insert_entry(entry.get_row_index(), column_.end()); } else { diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/list_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/list_column.h index 5793d5239..658541f97 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/list_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/list_column.h @@ -812,6 +812,8 @@ inline void List_column::push_back(const Entry& entry) { static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + GUDHI_CHECK(entry.get_row_index() > get_pivot(), "The new row index has to be higher than the current pivot."); + if constexpr (Master_matrix::Option_list::is_z2) { _insert_entry(entry.get_row_index(), column_.end()); } else { diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/naive_vector_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/naive_vector_column.h index 21809f121..e99d37b7e 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/naive_vector_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/naive_vector_column.h @@ -808,6 +808,8 @@ inline void Naive_vector_column::push_back(const Entry& entry) { static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + GUDHI_CHECK(entry.get_row_index() > get_pivot(), "The new row index has to be higher than the current pivot."); + if constexpr (Master_matrix::Option_list::is_z2) { _insert_entry(entry.get_row_index(), column_); } else { diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/set_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/set_column.h index c2efb18b2..6f69c53c9 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/set_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/set_column.h @@ -804,6 +804,8 @@ inline void Set_column::push_back(const Entry& entry) { static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + GUDHI_CHECK(entry.get_row_index() > get_pivot(), "The new row index has to be higher than the current pivot."); + if constexpr (Master_matrix::Option_list::is_z2) { _insert_entry(entry.get_row_index(), column_.end()); } else { diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/unordered_set_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/unordered_set_column.h index 9513a2022..b63adcbac 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/unordered_set_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/unordered_set_column.h @@ -807,6 +807,8 @@ inline void Unordered_set_column::push_back(const Entry& entry) { static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + GUDHI_CHECK(entry.get_row_index() > get_pivot(), "The new row index has to be higher than the current pivot."); + if constexpr (Master_matrix::Option_list::is_z2) { _insert_entry(entry.get_row_index()); } else { diff --git a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/vector_column.h b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/vector_column.h index 0721d86ed..f6a7da8e8 100644 --- a/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/vector_column.h +++ b/src/Persistence_matrix/include/gudhi/Persistence_matrix/columns/vector_column.h @@ -910,6 +910,8 @@ inline void Vector_column::push_back(const Entry& entry) { static_assert(Master_matrix::Option_list::is_of_boundary_type, "`push_back` is not available for Chain matrices."); + GUDHI_CHECK(entry.get_row_index() > get_pivot(), "The new row index has to be higher than the current pivot."); + if constexpr (Master_matrix::Option_list::is_z2) { _insert_entry(entry.get_row_index(), column_); } else {