Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function to restrict matrices #14096

Merged
merged 1 commit into from Sep 8, 2022
Merged

Conversation

peterrum
Copy link
Member

@peterrum peterrum commented Jul 5, 2022

This PR introduces functions that allow to extract parts of a distributed sparse matrix. In the context of Schwarz and Domain Decomposition methods this is known as restriction: A_i = R_i A R_i^T, where R_i is Boolean matrix. The granularity of the restriction is either on a cell level (restrict_to_cells()) or can be specified by an index set (restrict_to_serial_sparse_matrix()).

I have create a new namespace with the name SparseMatrixTools. Is there already a more suitable place for such functionality?

Comment on lines 31 to 35
template <typename T>
std::tuple<T, T>
prefix_sum(const T & value,
const MPI_Comm &comm,
const bool exclusive = true)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kronbichler Do you thinks such a function would be helpful? We have used prefix sums already at so many places! If yes, I would generalize the function so that it also works for vectors (as we need to process data for all multigrid levels simultaneously).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that function would make sense, yes.

@drwells
Copy link
Member

drwells commented Jul 5, 2022

Could we put these in MatrixTools instead? We don't use that namespace much but I would prefer not to introduce a new one that is similar.

@peterrum
Copy link
Member Author

peterrum commented Jul 5, 2022

Could we put these in MatrixTools instead? We don't use that namespace much but I would prefer not to introduce a new one that is similar.

I know the MatrixTools and MatrixCreator namespace. But both are in the numerics folder (which does not fit for the current use case) and the purpose of the functions are somewhat different (modify the matrix according to some boundary condition and create a matrix for a special type of operator (mass matrix/Laplace matrix)).

@kronbichler
Copy link
Member

Regarding the namespace, I have no strong opinion. SparseMatrixTools sounds OK in my opinion. One question I'm wondering, without having looked at it: Did you compare this to the sparse_vanka.h file? I would assume something similar to happen there, but the last time I looked in that file was in 2009.

@peterrum
Copy link
Member Author

Regarding the namespace, I have no strong opinion. SparseMatrixTools sounds OK in my opinion. One question I'm wondering, without having looked at it: Did you compare this to the sparse_vanka.h file? I would assume something similar to happen there, but the last time I looked in that file was in 2009.

@kronbichler I went through the SparseVanka code. It is serial and I fail how it would help. Indeed, the code extracts submatrices from a (serial) sparse matrix:

inverses[row]->extract_submatrix_from(*matrix, local_indices, local_indices);

which is the following way:

template <typename number>
template <typename MatrixType, typename index_type>
inline void
FullMatrix<number>::extract_submatrix_from(
const MatrixType & matrix,
const std::vector<index_type> &row_index_set,
const std::vector<index_type> &column_index_set)
{
AssertDimension(row_index_set.size(), this->n_rows());
AssertDimension(column_index_set.size(), this->n_cols());
const size_type n_rows_submatrix = row_index_set.size();
const size_type n_cols_submatrix = column_index_set.size();
for (size_type sub_row = 0; sub_row < n_rows_submatrix; ++sub_row)
for (size_type sub_col = 0; sub_col < n_cols_submatrix; ++sub_col)
(*this)(sub_row, sub_col) =
matrix.el(row_index_set[sub_row], column_index_set[sub_col]);
}

That code, however, is not able to handle the two major issues tackled in the new functions in this PR:

* @note In a certain sense, this is the reversion of the cell loop during
* matrix assembly. However, doing this on a distributed matrix is not
* trivial, since 1) rows might be owned by different processes and 2) degrees
* of freedoms might be constrained, resulting in "missing" entries in the
* matrix.

@peterrum
Copy link
Member Author

peterrum commented Aug 4, 2022

I have updated PR and removed the dependency to #14124. Ready from my side.

@peterrum peterrum force-pushed the sparse_matrix_tools branch 3 times, most recently from 8906844 to 0e49f20 Compare August 7, 2022 18:42
include/deal.II/lac/sparse_matrix_tools.h Outdated Show resolved Hide resolved
* is performed to iterativly solve a system of type $Au=f$.
*
* @warning This is a collective call that needs to be executed by all
* processes in the communicator.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain which communicator you refer to? The one of the system_matrix?

include/deal.II/lac/sparse_matrix_tools.h Outdated Show resolved Hide resolved
include/deal.II/lac/sparse_matrix_tools.h Outdated Show resolved Hide resolved
MPI_Comm
get_mpi_communicator(const SparseMatrixType &sparse_matrix)
{
return sparse_matrix.get_mpi_communicator();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be adjusted, because we also have ChunkSparseMatrix and some similar classes, that are similar to dealii::SparseMatrix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

include/deal.II/lac/sparse_matrix_tools.h Outdated Show resolved Hide resolved
include/deal.II/lac/sparse_matrix_tools.h Outdated Show resolved Hide resolved
Comment on lines +499 to +528
if (indices[c].size() == 0)
continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be documented - we support the case of empty indices, in which case the result is also empty (clearly).

@peterrum
Copy link
Member Author

peterrum commented Sep 3, 2022

@kronbichler I have fixed the typos and updated the documentation.

@drwells drwells merged commit 4fdcac0 into dealii:master Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants