-
Notifications
You must be signed in to change notification settings - Fork 91
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
Faster construction of sparse block matrices #1326
Conversation
This is in preparation for the introduciton of a corresponding function for creating sparse matrices from sparse blocks
This is equivalent to, but faster than, sps.block_diag. Also added a test
The method is faster than creating a csr-matrix or other alternatives Also test
This should give notable computational savings
Mocked discretization matrices should be sparse to comply with the real framework to be tested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not review the entire code (yet). Had only a look so far at the main methods, which seemed good to me so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in the main. Some questions and suggestions.
Great change! As far as I understood, this will fix #1084. |
Co-authored-by: Ivar Stefansson <ivar.stefansson@uib.no>
Apparently, mypy needs this
@IvarStefansson Thanks for the review. Most things should be taken care of now, though I left some comments for you to resolve. From your comments, and my own reading of the code, it is clear that it is easy to misinterpret the different approaches to constructing sparse block matrices - it is actually necessary to read the documentation to understand what each function does. I tried to improve the documentation, without overemphasizing this point. Please have a look at this in particular. EDIT: I upgraded the documentation of the functions with better descriptions, and inserted PS: I'm also fighting mypy. Don't ask. |
Yes, it should at least take away the main part of the cost. |
Proposed changes
This PR introduces helper functions for fast construction of sparse block-diagonal matrices, of
csr
,csc
anddia
form. This allows us to avoid callingscipy.sparse.block_diag
, which, though great, also takes account for all matter of edge cases, and is therefore slow. The new functions have been taken into use throughout the code.Profiling (by line_profiler) indicates computational savings of ~90% on the construction of block diagonal discretization matrices under Ad parsing (which again consumed ~1/3 of all parsing time) on a 3d poromechanical case with a relatively coarse grid and not too many fractures. I do not know how this translates to other cases, but expect the speedup to be non-negligible in many cases.
I also renamed
matrix_operations.csr_matrix_from_blocks
->matrix_operations.csr_matrix_from_dense_blocks
, to enable more natural names for the new functioncsr_matrix_from_sparse_blocks
(and similar forcsc
).Tests are introduced, tests for
csr_matrix_from_dense_blocks
has been consolidated somewhat.Resolves #1084.
Types of changes
What types of changes does this PR introduce to PorePy?
Put an
x
in the boxes that apply.Checklist
Put an
x
in the boxes that apply or explain briefly why the box is not relevant.pytest
was run with the--run-skipped
flag.