Introduce matrix slicer objects #1346
Conversation
Also renamed slice_mat -> slice_sparse_matrix
…ws/columns Changed the test parameters somewhat, but the effective test coverage should be the same
First attempt, improvements to come.
The method for slicing matrices was changed some commits ago, but this line was not updated
When the projection is onto the range, native scipy slicing is much more efficient than the tailored approach
The new definition is consistent with the representation of the slicer as a projection matrix Also updated the test
The main improvement is to let the right multiplication and division method return a copy of the object
For some reason, one of the lists of tested and testable methods has been shuffled. This is potentially because the order in which the constitutive laws are accessed has changed (due to changes done during introduction of the MatrixSlicer, though EK does not understand exactly how). There is no reason why the two lists should be arranged identically, so sorting should be okay.
This should be significantly faster than the old sparse matrix constructed through a Kronecker product
… in basis vectors
Transpose method copies array indices. Fixed missing argument in copy method
This reduces the chances of unintended edits of the underlying objects
This was needed to use Ad basis functions on both vectors and scalars Also improved test
|
Thanks for the thorough review. I believe I have addressed most of the comments now, most of those not yet resolved can hopefully be put to rest quite soon. In terms of functionality, the main change since the initial PR is that the You suggest that we somehow force the test of |
This works through the system of delayed evaluation in the slicer
This is implemented through delayed evaluation
|
Ready for re-review. Main changes:
|
Co-authored-by: Ivar Stefansson <[email protected]>
Proposed changes
This PR contains two main changes:
MatrixSlicerclass is introduced as an alternative to creating projection matrices. Benchmarking indicates that, depending on the context, projection by the slicer is newer worse that the equivalent matrix, and it can be orders of magnitudes faster. For the subdomain and mortar projections (not yet introduced, see below), it is reasonable to expect decent performance gains of 10s of percentages.pp.ad.Projectionis introduced as a wrapper of theMatrixSlicerin the ad framework. ThisProjectionclass has been taken into use in thegeometry.e_i()method, with significant performance improvements (cut estimated to >90% of the computation time in some cases). Since theMatrixSliceronly supports a limited set of arithmetic operations, it was necessary to introduce changes to the way basis functions are summed (see new methodpp.ad.sum_projection_list) and parsed (see changes toAdParser).In addition, I have done some maintenance, mainly to
matrix_operations.pyand the associated tests. These files would benefit from further improvements in that regard, but I chose not to do so here.What has not been done
I have not introduced the new projections in
SubdomainProjectionandMortarProjection. During development and benchmarking, and partly triggered by #1343, it became clear that we need to do a more thorough rethinking of how projections are designed (see #1345), and I therefore chose to go for a PR with limited scope.Questions to reviewers
MatrixSlicerbe available for direct import,pp.MatrixSlicer? I'm not sure how much we will use it directly. Typing would be more elegant, though.MatrixSliceran okay name? It can slice more than matrices.EDIT: In light of the centralized projection storage discussed in #1346,
pp.ad.Projectionmay not be the best name for the class mentioned in point 2 in the above list. Naming suggestions are much appreciated.Types of changes
What types of changes does this PR introduce to PorePy?
Put an
xin the boxes that apply.Checklist
Put an
xin the boxes that apply or explain briefly why the box is not relevant.pytestwas run with the--run-skippedflag.