Let PCASM refresh its own submatrices - #5403
Open
pbrubeck wants to merge 1 commit into
Open
Conversation
pbrubeck
marked this pull request as ready for review
August 28, 2026 00:51
pbrubeck
force-pushed
the
pbrubeck/fix-pcasm-getsubksp-before-setup
branch
from
August 28, 2026 07:43
bee0dce to
1159780
Compare
pbrubeck
changed the title
pbrubeck
force-pushed
the
pbrubeck/fix-pcasm-getsubksp-before-setup
branch
2 times, most recently
from
August 28, 2026 07:48
8546465 to
7fd7b8d
Compare
ASMPatchPC.update cleared the factored flag a subsolver's in-place factorization leaves on its submatrix, by iterating the sub-KSPs of the inner PCASM. That is PCASM's own business, and doing it here crashed: for the petscasm backend initialize only specifies the subdomains, so on a setup that precedes any apply the sub-KSPs do not exist yet and PCASMGetSubKSP hands back a null array. PCSetUp_ASM now calls MatSetUnfactored on the submatrices it reuses, so update has nothing left to do. Add a regression test: a star patch whose subsolver factors in place, solved twice with a changed coefficient, at 1 and 2 processes.
pbrubeck
force-pushed
the
pbrubeck/fix-pcasm-getsubksp-before-setup
branch
from
August 28, 2026 08:10
7fd7b8d to
70336ba
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
1 participant
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.
ASMStarPChands its patches to PETSc'sPCASM, which keeps one small matrix per patch. If the patch solver is told to factor in place, it overwrites that little matrix with its own factors. So the next time the operator changes and PETSc goes to refill the patch matrices, it refuses: as far as it can tell, that matrix is a factorization, not a matrix.Firedrake worked around this by reaching into
PCASM, walking its sub-solvers, and clearing the "this is factored" flag itself. That is whatupdatewas for.The trouble is that Firedrake only tells
PCASMwhich patches it wants.PCASMdoes not create the sub-solvers until it is set up, and nothing here sets it up. Usually the first time the preconditioner is applied forces that, and by the timeupdateruns the sub-solvers exist. But if the preconditioner is set up twice before it is ever applied,updateasks for sub-solvers that were never created and gets a null pointer back, which segfaults. A multigrid smoother does exactly that from three levels up, where the level's operator is swapped out after the initial setup.The companion PETSc MR makes
PCASMclear the flag itself when it refills the patch matrices, which is where the job belonged. That leaves nothing forupdateto do.Changes
ASMPatchPC.updateis now empty. It cannot be deleted outright, becausePCBase.updateis anabc.abstractmethod.test_star_inplace_factorization_reused, covering a patch solver that factors in place across a change of coefficient.AI was used to draft this change (Claude Code, Opus 5).