Skip to content

Commit

Permalink
adaptation of the indexation options. But now, unit tests for id chai…
Browse files Browse the repository at this point in the history 
…n matrices are missing, the ones available are for container indexation. As the id interface is not usefull yet, to do later.
  • Loading branch information
@hschreiber
hschreiber committed Feb 19, 2024
1 parent a6aace5 commit c25ca91
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 59 deletions.
105 changes: 71 additions & 34 deletions src/Persistence_matrix/include/gudhi/matrix.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ class Matrix {
Options::column_type == Column_types::HEAP,
Heap_column_type,
typename std::conditional<
Options::column_type == Column_types::LIST,
Options::column_type == Column_types::LIST,
List_column_type,
typename std::conditional<
Options::column_type == Column_types::SET,
Expand Down Expand Up @@ -932,27 +932,36 @@ class Matrix {
const cycle_type& get_representative_cycle(const Bar& bar);

private:
using matrix_type = typename std::conditional<
isNonBasic,
typename std::conditional<
Options::is_of_boundary_type,
typename std::conditional<
Options::has_vine_update || Options::can_retrieve_representative_cycles,
typename std::conditional<Options::is_indexed_by_position,
RU_matrix_type,
Id_to_index_overlay<RU_matrix_type, Matrix<Options> >
>::type,
typename std::conditional<Options::is_indexed_by_position,
Boundary_matrix_type,
Id_to_index_overlay<Boundary_matrix_type, Matrix<Options>>
>::type
>::type,
typename std::conditional<Options::is_indexed_by_position,
Position_to_index_overlay<Chain_matrix_type, Matrix<Options>>,
Chain_matrix_type
>::type
>::type,
Base_matrix_type
using matrix_type =
typename std::conditional<
isNonBasic,
typename std::conditional<
Options::is_of_boundary_type,
typename std::conditional<
Options::has_vine_update || Options::can_retrieve_representative_cycles,
typename std::conditional<
Options::column_indexation_type == Column_indexation_types::CONTAINER ||
Options::column_indexation_type == Column_indexation_types::POSITION,
RU_matrix_type, Id_to_index_overlay<RU_matrix_type, Matrix<Options> >
>::type,
typename std::conditional<
Options::column_indexation_type == Column_indexation_types::CONTAINER ||
Options::column_indexation_type == Column_indexation_types::POSITION,
Boundary_matrix_type,
Id_to_index_overlay<Boundary_matrix_type, Matrix<Options> >
>::type
>::type,
typename std::conditional<
Options::column_indexation_type == Column_indexation_types::CONTAINER,
Chain_matrix_type,
typename std::conditional<
Options::column_indexation_type == Column_indexation_types::POSITION,
Position_to_index_overlay<Chain_matrix_type, Matrix<Options> >,
Id_to_index_overlay<Chain_matrix_type, Matrix<Options> >
>::type
>::type
>::type,
Base_matrix_type
>::type;

matrix_type matrix_;
Expand Down Expand Up @@ -1106,10 +1115,11 @@ inline const typename Matrix<Options>::Column_type& Matrix<Options>::get_column(

template <class Options>
inline const typename Matrix<Options>::Column_type& Matrix<Options>::get_column(index columnIndex, bool inR) {
//TODO: I don't think there is a particular reason why the indexation is forced, should be removed.
static_assert(isNonBasic &&
Options::is_of_boundary_type &&
(Options::has_vine_update || Options::can_retrieve_representative_cycles) &&
Options::is_indexed_by_position,
Options::column_indexation_type != Column_indexation_types::IDENTIFIER,
"Only enabled for position indexed RU matrices.");

return matrix_.get_column(columnIndex, inR);
Expand All @@ -1132,9 +1142,10 @@ inline const typename Matrix<Options>::Row_type& Matrix<Options>::get_row(id_ind
template <class Options>
inline const typename Matrix<Options>::Row_type& Matrix<Options>::get_row(id_index rowIndex, bool inR) {
static_assert(Options::has_row_access, "'get_row' is not available for the chosen options.");
//TODO: I don't think there is a particular reason why the indexation is forced, should be removed.
static_assert(isNonBasic && Options::is_of_boundary_type &&
(Options::has_vine_update || Options::can_retrieve_representative_cycles) &&
Options::is_indexed_by_position,
Options::column_indexation_type != Column_indexation_types::IDENTIFIER,
"Only enabled for position indexed RU matrices.");

return matrix_.get_row(rowIndex, inR);
Expand Down Expand Up @@ -1278,10 +1289,11 @@ inline void Matrix<Options>::zero_cell(index columnIndex, id_index rowIndex) {

template <class Options>
inline void Matrix<Options>::zero_cell(index columnIndex, id_index rowIndex, bool inR) {
//TODO: I don't think there is a particular reason why the indexation is forced, should be removed.
static_assert(isNonBasic &&
Options::is_of_boundary_type &&
(Options::has_vine_update || Options::can_retrieve_representative_cycles) &&
Options::is_indexed_by_position,
Options::column_indexation_type != Column_indexation_types::IDENTIFIER,
"Only enabled for RU matrices.");

return matrix_.zero_cell(columnIndex, rowIndex, inR);
Expand All @@ -1297,9 +1309,10 @@ inline void Matrix<Options>::zero_column(index columnIndex) {

template <class Options>
inline void Matrix<Options>::zero_column(index columnIndex, bool inR) {
//TODO: I don't think there is a particular reason why the indexation is forced, should be removed.
static_assert(isNonBasic && Options::is_of_boundary_type &&
(Options::has_vine_update || Options::can_retrieve_representative_cycles) &&
Options::is_indexed_by_position,
Options::column_indexation_type != Column_indexation_types::IDENTIFIER,
"Only enabled for RU matrices.");

return matrix_.zero_column(columnIndex, inR);
Expand All @@ -1312,9 +1325,10 @@ inline bool Matrix<Options>::is_zero_cell(index columnIndex, id_index rowIndex)

template <class Options>
inline bool Matrix<Options>::is_zero_cell(index columnIndex, id_index rowIndex, bool inR) const {
//TODO: I don't think there is a particular reason why the indexation is forced, should be removed.
static_assert(isNonBasic && Options::is_of_boundary_type &&
(Options::has_vine_update || Options::can_retrieve_representative_cycles) &&
Options::is_indexed_by_position,
Options::column_indexation_type != Column_indexation_types::IDENTIFIER,
"Only enabled for RU matrices.");

return matrix_.is_zero_cell(columnIndex, rowIndex, inR);
Expand All @@ -1327,9 +1341,10 @@ inline bool Matrix<Options>::is_zero_column(index columnIndex) {

template <class Options>
inline bool Matrix<Options>::is_zero_column(index columnIndex, bool inR) {
//TODO: I don't think there is a particular reason why the indexation is forced, should be removed.
static_assert(isNonBasic && Options::is_of_boundary_type &&
(Options::has_vine_update || Options::can_retrieve_representative_cycles) &&
Options::is_indexed_by_position,
Options::column_indexation_type != Column_indexation_types::IDENTIFIER,
"Only enabled for RU matrices.");

return matrix_.is_zero_column(columnIndex, inR);
Expand Down Expand Up @@ -1384,7 +1399,8 @@ inline const typename Matrix<Options>::barcode_type& Matrix<Options>::get_curren
template <class Options>
inline void Matrix<Options>::swap_columns(index columnIndex1, index columnIndex2) {
static_assert((!isNonBasic && !Options::has_column_compression) ||
(isNonBasic && Options::is_of_boundary_type && Options::is_indexed_by_position &&
(isNonBasic && Options::is_of_boundary_type &&
Options::column_indexation_type != Column_indexation_types::IDENTIFIER &&
!Options::has_vine_update && !Options::can_retrieve_representative_cycles),
"This method was not enabled.");
return matrix_.swap_columns(columnIndex1, columnIndex2);
Expand All @@ -1393,7 +1409,8 @@ inline void Matrix<Options>::swap_columns(index columnIndex1, index columnIndex2
template <class Options>
inline void Matrix<Options>::swap_rows(index rowIndex1, index rowIndex2) {
static_assert((!isNonBasic && !Options::has_column_compression) ||
(isNonBasic && Options::is_of_boundary_type && Options::is_indexed_by_position &&
(isNonBasic && Options::is_of_boundary_type &&
Options::column_indexation_type != Column_indexation_types::IDENTIFIER &&
!Options::has_vine_update && !Options::can_retrieve_representative_cycles),
"This method was not enabled.");
return matrix_.swap_rows(rowIndex1, rowIndex2);
Expand All @@ -1410,26 +1427,46 @@ inline void Matrix<Options>::swap_at_indices(index index1, index index2) {

template <class Options>
inline bool Matrix<Options>::vine_swap_with_z_eq_1_case(pos_index index) {
static_assert(Options::has_vine_update && Options::is_indexed_by_position, "This method was not enabled.");
static_assert(Options::has_vine_update,
"This method was not enabled.");
static_assert(Options::column_indexation_type == Column_indexation_types::POSITION ||
(Options::is_of_boundary_type &&
Options::column_indexation_type == Column_indexation_types::CONTAINER),
"This method was not enabled.");
return matrix_.vine_swap_with_z_eq_1_case(index);
}

template <class Options>
inline typename Matrix<Options>::index Matrix<Options>::vine_swap_with_z_eq_1_case(index columnIndex1,
index columnIndex2) {
static_assert(Options::has_vine_update && !Options::is_indexed_by_position, "This method was not enabled.");
static_assert(Options::has_vine_update,
"This method was not enabled.");
static_assert(Options::column_indexation_type == Column_indexation_types::IDENTIFIER ||
(!Options::is_of_boundary_type &&
Options::column_indexation_type == Column_indexation_types::CONTAINER),
"This method was not enabled.");
return matrix_.vine_swap_with_z_eq_1_case(columnIndex1, columnIndex2);
}

template <class Options>
inline bool Matrix<Options>::vine_swap(pos_index index) {
static_assert(Options::has_vine_update && Options::is_indexed_by_position, "This method was not enabled.");
static_assert(Options::has_vine_update,
"This method was not enabled.");
static_assert(Options::column_indexation_type == Column_indexation_types::POSITION ||
(Options::is_of_boundary_type &&
Options::column_indexation_type == Column_indexation_types::CONTAINER),
"This method was not enabled.");
return matrix_.vine_swap(index);
}

template <class Options>
inline typename Matrix<Options>::index Matrix<Options>::vine_swap(index columnIndex1, index columnIndex2) {
static_assert(Options::has_vine_update && !Options::is_indexed_by_position, "This method was not enabled.");
static_assert(Options::has_vine_update,
"This method was not enabled.");
static_assert(Options::column_indexation_type == Column_indexation_types::IDENTIFIER ||
(!Options::is_of_boundary_type &&
Options::column_indexation_type == Column_indexation_types::CONTAINER),
"This method was not enabled.");
return matrix_.vine_swap(columnIndex1, columnIndex2);
}

Expand Down
11 changes: 8 additions & 3 deletions src/Persistence_matrix/include/gudhi/persistence_matrix_options.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ enum Column_types {
INTRUSIVE_SET
};

enum Column_indexation_types {
CONTAINER,
POSITION,
IDENTIFIER
};

template<bool is_z2_only = true, class Field_type = Z2_field_element, Column_types col_type = Column_types::INTRUSIVE_SET, bool parallelizable = false>
struct Default_options{
using field_coeff_type = Field_type;
Expand All @@ -44,6 +50,8 @@ struct Default_options{
static const bool is_z2 = is_z2_only;
static const Column_types column_type = col_type;

static const Column_indexation_types column_indexation_type = Column_indexation_types::CONTAINER;

static const bool is_separated_by_dimension = false; //not implemented yet
static const bool is_parallelizable = parallelizable; //not implemented yet
static const bool is_double_linked = true; //not implemented yet, it depends of the column type for now (all double linked except for UNORDERED_SET). usefull?
Expand All @@ -58,8 +66,6 @@ struct Default_options{
static const bool has_map_column_container = false;
static const bool has_removable_columns = false;
static const bool is_of_boundary_type = true; //ignored if not at least one specialised method is enabled: has_column_pairings, has_vine_update, can_retrieve_representative_cycles
//rename to columns_are_indexed_by_position
static const bool is_indexed_by_position = is_of_boundary_type; //useless if has_vine_update = false, as the two indexing strategies only differ when swaps occur.
static const bool has_column_compression = false; //can be enabled only if no specialised method is enabled: has_column_pairings, has_vine_update, can_retrieve_representative_cycles, has_map_column_container
static const bool has_column_and_row_swaps = false; //ignored if has_vine_update or can_retrieve_representative_cycles is true.
};
Expand All @@ -70,7 +76,6 @@ struct Zigzag_options : Default_options<true, Z2_field_element, column_type, par
static const bool has_column_pairings = false;
static const bool has_vine_update = true;
static const bool is_of_boundary_type = false;
static const bool is_indexed_by_position = false;
static const bool has_map_column_container = true;
static const bool has_removable_rows = true;
};
Expand Down
12 changes: 6 additions & 6 deletions src/Persistence_matrix/test/pm_matrix_tests.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ void test_chain_boundary_insertion(Matrix& m1, Matrix& m2){
auto orderedBoundaries = build_simple_boundary_matrix<typename Matrix::Column_type>();

for (unsigned int i = 0; i < orderedBoundaries.size(); ++i){
if constexpr (Matrix::Option_list::is_indexed_by_position) m1.insert_boundary(orderedBoundaries[i]);
if constexpr (is_indexed_by_position<Matrix>()) m1.insert_boundary(orderedBoundaries[i]);
else m1.insert_boundary(i, orderedBoundaries[i]);
}
BOOST_CHECK_EQUAL(m1.get_number_of_columns(), 7);
Expand Down Expand Up @@ -1326,7 +1326,7 @@ void test_ru_operation(){

test_content_equality(columns, m);
unsigned int i = 0;
if constexpr (Matrix::Option_list::is_indexed_by_position){
if constexpr (is_indexed_by_position<Matrix>()){
for (auto& b : uColumns){
test_column_equality<typename Matrix::Column_type>(b, get_column_content_via_iterators(m.get_column(i++, false)));
}
Expand All @@ -1344,7 +1344,7 @@ void test_ru_operation(){
uColumns[5] = {{3,2},{4,1},{5,4}};
}
test_content_equality(columns, m);
if constexpr (Matrix::Option_list::is_indexed_by_position){
if constexpr (is_indexed_by_position<Matrix>()){
i = 0;
for (auto& b : uColumns){
test_column_equality<typename Matrix::Column_type>(b, get_column_content_via_iterators(m.get_column(i++, false)));
Expand All @@ -1363,7 +1363,7 @@ void test_ru_operation(){
uColumns[5] = {{3,2},{4,2},{5,4}};
}
test_content_equality(columns, m);
if constexpr (Matrix::Option_list::is_indexed_by_position){
if constexpr (is_indexed_by_position<Matrix>()){
i = 0;
for (auto& b : uColumns){
test_column_equality<typename Matrix::Column_type>(b, get_column_content_via_iterators(m.get_column(i++, false)));
Expand All @@ -1380,7 +1380,7 @@ void test_ru_operation(){
uColumns[3] = {{4,2},{5,4}};
}
test_content_equality(columns, m);
if constexpr (Matrix::Option_list::is_indexed_by_position){
if constexpr (is_indexed_by_position<Matrix>()){
i = 0;
for (auto& b : uColumns){
test_column_equality<typename Matrix::Column_type>(b, get_column_content_via_iterators(m.get_column(i++, false)));
Expand All @@ -1396,7 +1396,7 @@ void test_ru_operation(){
uColumns[4] = {{3,3},{4,4},{5,1}};
}
test_content_equality(columns, m);
if constexpr (Matrix::Option_list::is_indexed_by_position){
if constexpr (is_indexed_by_position<Matrix>()){
i = 0;
for (auto& b : uColumns){
test_column_equality<typename Matrix::Column_type>(b, get_column_content_via_iterators(m.get_column(i++, false)));
Expand Down
Loading

0 comments on commit c25ca91

Please sign in to comment.