Make lu/qr of ComponentMatrix preserve the wrapper on factors - #391
Merged
ChrisRackauckas merged 1 commit intoJul 11, 2026
Merged
Conversation
LinearSolve >= 3.87 types its LUFactorization cache slot from ArrayInterface.lu_instance(A) but stores lu(A, check=false) when not aliasing A. For a ComponentMatrix, lu_instance returned an LU wrapping a ComponentMatrix while lu(A) demotes to Matrix (its copy goes through similar(A, T, dims), which drops the axes), so storing the factorization threw "TypeError: in setfield!" and broke the Downstream diffeq tests. Define lu(::ComponentMatrix) to factorize getdata(A) (LAPACK fast path) and rewrap the factors, so lu, lu! (already wrapper-preserving since ComponentMatrix is strided), and lu_instance all return the same type. Apply the same treatment to qr/qr_instance since LinearSolve's default solver falls back to QR on singular matrices and hits the identical mismatch there. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZG7J62sLGGMUmvNsKHVMA
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.
Note
Please ignore this PR until it has been reviewed by @ChrisRackauckas.
Fixes the Downstream CI failure seen in https://github.com/SciML/ComponentArrays.jl/actions/runs/29140401332/job/86512903689 (Issue 36 test in
test/Downstream/diffeq_tests.jl).Root cause
LinearSolve >= v3.87 (and v5) types its
LUFactorizationcache slot fromArrayInterface.lu_instance(A)but storeslu(A, check = false)when not aliasingA. For aComponentMatrix:ArrayInterface.lu_instance(::ComponentMatrix)(defined here) returns anLUwrapping a ComponentMatrix,lu!(A)also keeps the wrapper (ComponentMatrix is strided, so it hits LAPACK directly),lu(A)copies throughsimilar(A, T, dims), which drops the axes and demotes to a plain Matrix.Storing the
LU{..., Matrix, ...}into theLU{..., ComponentMatrix, ...}-typed slot throwsTypeError: in setfield!, which is exactly the downstream error:This reproduces without OrdinaryDiffEq via a plain
solve(LinearProblem(A, b))on a 2000x2000ComponentMatrix.Fix
Make
lu(::ComponentMatrix)factorizegetdata(A)(keeping the LAPACK fast path) and rewrap the factors, solu,lu!, andlu_instanceall return the same type. The same treatment is applied toqr/ArrayInterface.qr_instance, because LinearSolve's default solver falls back toQRFactorizationon singular matrices (safetyfallback) anddo_factorizationuses the in-placeqr!(A, pivot)there, which hits the identical mismatch (slot typedQRCompactWY{..., Matrix, ...}fromqr_instance, valueQRCompactWY{..., ComponentMatrix, ...}) — i.e. any ODE solve whose W matrix goes singular would crash instead of falling back.Note
GenericLUFactorization(chosen by the default solver for sizes <= 10) storesgeneric_lufact!(A), which wrapsAitself, so its slot genuinely must beLU{..., ComponentMatrix, ...}— that is why the fix preserves the wrapper everywhere rather than demotinglu_instancetoMatrix(which would just move the mismatch to the small-size path that currently passes).Verification (all run locally)
solve(LinearProblem(A, b))on a 2000x2000 ComponentMatrix: fails with the CI TypeError before, passes after (LinearSolve v5.0.0, Julia 1.12.6).alias_Ain (false, true): all pass.LUFactorization,GenericLUFactorization,QRFactorization,KrylovJL_GMRES: all pass.Successwith the least-squares fallback.typeof(f(A)) === typeof(f!(copy(A))) === typeof(instance(A))for lu and qr (NoPivot and ColumnNorm): verified on Julia 1.12.6 and 1.10.11.GROUP=Core Pkg.test()(Julia 1.12): pass, including new regression tests intest/math_tests.jl.GROUP=Downstream Pkg.test()(Julia 1.12): 6 passed, 1 broken (the broken one is the pre-existing@test_brokenSundials Issue 53), versus4 passed, 1 errored, 1 brokenon CI before this change.Runic has been run on all changed files. Version bumped to 0.15.42.
🤖 Generated with Claude Code
https://claude.ai/code/session_01PZG7J62sLGGMUmvNsKHVMA