Refactor LocalMatrix
Description
Local matrices are something which users are exposed to work with when implementing the local operators. However, it seems to me that it is not clear how to work with them beyond the accumulate
method. For instance, It took me a long time to realize one receives a WeightedMatrixAccumulationView
instead of a LocalMatrix
and it took me even longer to realize that in some cases one might receive AliasedMatrixView
instead. The local matrix interface that a user should follow to fit the different types is not documented nor the way they are used within PDELab.
Now, I want to add yet another one: SparseLocalMatrix
. If I just add it without organizing the problems I described above, the problem will become more apparent and it will just make switching between the different types even more difficult.
Proposal
- Rename
LocalMatrix
toDenseLocalMatrix
. Of course, using the corresponding deprecation warnings. - Define an interface suitable for the different implementations that a local matrix should follow and document it. This can be enforced using concepts which in case of a non-conforming interface should give a descriptive message to the user.
- Separate the weighted view from the
LocalMatrix
implementation. This is an unnecessary redundancy on the current implementation. Furthermore, doing so may allow using the weighted view regardless of the actual local matrix implementation. - Extend the
AliasedMatrixView
to also work with non-entity-blocked global matrices. This needs discussion with the FastDG guys (@dominic and @rhess) to see whether this is possible at all so that performance is not affected🚗 💨 . - Leave the user to choose the type of local matrix to use. This ideally would happen at
GridOperator
level but we already have quite a bulky signature so I hear offers for other options👂 .
How to test the implementation?
- All current tests should keep passing after the refactoring.