Introduce diagonal ad arrays - #1755
Draft
keileg wants to merge 289 commits into
Draft
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
keileg
marked this pull request as draft
August 27, 2026 10:10
Need this to deal with composite operators that contain projections between different domains
Needed for OperatorSpace parsing
Needed for OperatorSpaces construction
Needed to expand to Nd
No need to override the superclass implementations
Use domains instead
Use the dof-information of the operator_range instead
Access through the operator_range instead
Use domains[0] instead
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
These methods are unlikely to ever change and do not need 400 lines of dedicated tests
The deleted lines are also present further up in the file
The power was included twice
…th diagonal ad arrays
Also cover diagonal ad arrays
This was a leftover from a previous implementation effort
The variables are known to be single (2d) arrays, so there is no need to stack them and thereby create a copy
It is no longer used in the production code, but still useful in tests. It can be purged from there, but that should be a part of a larger refactoring of that file.
Avoids branching on AdArray / DiagonalAdArray
keileg
force-pushed
the
diag_ad_array
branch
from
September 8, 2026 12:41
18eeb89 to
7eeee0d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
This PR implements a dense-array backend for forward Ad assembly. The idea is that, for quantities that are dependent on cell center quantities only (the idea can be generalized, but cell center values seemed like a reasonable compromise between generality and simplicity in implementation), derivatives can be represented by 2d dense numpy array instead of general scipy sparse matrices. A new class,
DiagonalAdArrayis introduced to represent such quantities.The point is that, in forward mode Ad, when parsing expressions that consist of only
DiagonalAdArrays, the rules of differentiation will be applied on numpy dense arrays, which can be significantly faster than dealing with sparse arrays (see benchmarking results below). The result from combining twoDiagonalAdArrays in an arithmetic operation is a newDiagonalAdArray, hence for complex expressions the computational gain will accumulate. When aDiagonalAdArrayis hit with a projection or a differential operator, it is transformed to a standardAdArrayand the old Ad parsing machinery takes over.See #1497 for more information and this outline for the original idea.
Scope
Though it is possible to extend the idea beyond expressions depending on cell center values, and hence delay the transformation to usual
AdArrays, I have chosen not to do so. To limit technicalities in the implementation, we also initialize aDiagonalAdArrayonly for scalar quantities (displacement does not qualify) defined on all subdomains. To be clear, in a displacement-pressure system, the pressure can still be represented as aDiagonalAdArray. Interface variables are not converted.I have not explored what it will take to go beyond these restrictions, but much of the parsing of the compositional models (which are the ones suffering the most from slow assembly) should be covered, so this should be a step in the right direction. If the results are positive, I am open for expanding the coverage of the idea in follow-up projects, but that should be based on identification of bottlenecks based on benchmarking.
Benchmarking
I am 100% sure the results are in the right direction. The actual numbers produced below may partly be setup dependent, but the overall results should be fairly solid.
x+yand similar), diagonal arrays give a speedup of around 4-5 for arrays with size 10^5-10^6. The speedup seems to stabilize if the size is increased further. For smaller arrays (10^2-10^3), the speedup is much higher (~30), but this matters less.tests/functional/test_buoyancy_flow.pyas the starting point for benchmarking) I got a speedup of between 2 and 3 in the total time spent on assembly. Two comments:DiagonalAdArraysand expressions that are not suited for diagonal representation. I have not estimated the fraction of the computations that were amenable to diagonal representation, but the reduced gain compared to the forward mode evaluation can at least in part be attributed to this.Other changes
heavisideetc. now have systematic testing.Note that this PR is put on top of #1747 and thus seemingly contains substantial changes beyond what is described above. The PR will remain a draft until the upstream PR is merged.
Help needed
Though everything seems to be working, it would be great to have some QC on code robustness beyond what is currently available in the tests. In particular, a confirmation that the update does not break complex application code and that the timings broadly apply also in these cases. @mikeljordan @vlipovac: Would it be possible for you to run some of your simulations at some point and see what happens. It is not urgent since we cannot really move before #1747 is merged (also note that, to the degree you define your own equations on Ad operator form, that PR may introduce breaking changes - it should not be difficult to fix this, but an update will be needed).
Resolves #1497.
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.