Allow-list QRCompactWY instead of avoiding internal QR types - #392
Merged
ChrisRackauckas merged 1 commit intoJul 30, 2026
Merged
Conversation
Member
Author
|
Audit scratchpad for commit
No test was skipped or weakened. |
Rewrap the QR factorization by concrete type again so `qr`/`qr_instance` on a ComponentMatrix keep the wrapper on the factors, and add `:QRCompactWY` to the QA ExplicitImports ignore list rather than routing around the non-public name. Also cover the generic `QR` path so non-BLAS eltypes (BigFloat) keep the wrapper too. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
ChrisRackauckas-Claude
force-pushed
the
agent/fix-qr-public-api
branch
from
July 30, 2026 10:38
18d0735 to
3f49005
Compare
ChrisRackauckas-Claude
changed the title
ChrisRackauckas
marked this pull request as ready for review
July 30, 2026 10:40
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
2 participants
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.
Ignore this PR until it has been reviewed by @ChrisRackauckas.
What changed
Rebased on
mainand inverted the approach: instead of routing aroundLinearAlgebra.QRCompactWY, use it directly and mark it as allowed in the QA config.LinearAlgebra.qr(::ComponentMatrix)andArrayInterface.qr_instance(::ComponentMatrix)(reverts the workaround merged in Upgrade ComponentArrays QA to SciMLTesting 2.4 #394):QRCompactWYto theall_qualified_accesses_are_publicignore list intest/qa/qa.jl_rewrap_qr(::LinearAlgebra.QR, ax)branch so non-BLAS eltypes (e.g.BigFloat) keep theComponentMatrixwrapper too, with a regression testWhy
QRCompactWYis genuinely not public LinearAlgebra API, but rewrapping by concrete factorization type is the direct way to keep theComponentMatrixwrapper on the factors. Allow-listing the one non-public name is simpler than theqr!-on-a-0x0-instance indirection.Verified locally that the allow-list entry is load-bearing: with it removed, QA errors with
Local checks (Julia 1.12.4)
GROUP=QA: 19 passed, 0 failedGROUP=Core: passed;math_tests.jl102/102--checkGROUP=Downstream: 23 passed, 1 broken (the pre-existing@test_brokenattest/Downstream/diffeq_tests.jl:31)Please ignore this draft until it has been reviewed by @ChrisRackauckas.