Repository navigation
Split reflectorApply! into a dot-based and a loop-based kernel - #1735
Merged
Merged
Conversation
`reflectorApply!(x, τ, A)` was based on `dot` and `axpy!`, which do not treat the elements of `A` as opaque: for arrays of arrays, `dot` recurses into the elements. The computation is now carried out by one of two kernels, `reflectorApplyNumeric!` (the previous implementation) for numeric elements, and `reflectorApplyLoop!` with explicit loops otherwise. Packages may dispatch to either kernel for their own element types. With this, the packed `QR` solves and the multiplication by `QRPackedQ` work for right-hand sides with vector-valued elements. The padding in the wide QR solve is filled with `fill!`, since broadcasting an array-valued zero would recurse into it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rqqAa24hUALGk57NefU4G
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1735 +/- ##
=======================================
Coverage 97.09% 97.10%
=======================================
Files 35 35
Lines 17255 17277 +22
=======================================
+ Hits 16754 16777 +23
+ Misses 501 500 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 of 2 tasks
dkarrasch
commented
Oct 4, 2026
dkarrasch
commented
Oct 4, 2026
dkarrasch
commented
Oct 4, 2026
Co-authored-by: Daniel Karrasch <daniel.karrasch@posteo.de>
dkarrasch
pushed a commit
that referenced
this pull request
Oct 4, 2026
…or-valued right-hand side These are the remaining paths through `ldiv`; the least-squares case is supported since #1735. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rqqAa24hUALGk57NefU4G
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.
Follow-up to #1734. The left-applying
reflectorApply!(x, τ, A)useddotandaxpy!, so for arrays of arraysdotrecursed into the elements and the packedQRsolves failed for vector-valued right-hand sides. The computation is now carried out by one of two kernels:reflectorApplyNumeric!: the previousdot/axpy!implementation, used for numericxandA;reflectorApplyLoop!: explicit loops that only multiply the elements ofAby the entries ofxand byτ, used otherwise.reflectorApply!dispatches between them on the element types, and packages can add methods for their own types that dispatch to either kernel. The wide packed QR solve now fills its padding withfill!, since broadcasting an array-valued zero recurses into it as well. Numeric code paths are unchanged (timings ofqrfactUnblocked!and_lqmul!on master and this branch agree within noise).Tests: both kernels agree with each other and with the definition for
Float64,ComplexF64andQuaternionelements; a matrix ofSizedArrays is reflected componentwise; the@test_brokenfrom #1734 is replaced by solves andQ/Q'multiplications withSizedArrayright-hand sides for all three QR types, with square, tall and wide matrices, vector and matrix right-hand sides.Related: #904, #1114.
🤖 Generated with Claude Code
https://claude.ai/code/session_013rqqAa24hUALGk57NefU4G
Generated by Claude Code