Skip to content

Split reflectorApply! into a dot-based and a loop-based kernel - #1735

Merged
dkarrasch merged 3 commits into
masterfrom
claude/elegant-ramanujan-5hvsgj-reflector
Oct 4, 2026
Merged

dkarrasch merged 3 commits into
masterfrom
claude/elegant-ramanujan-5hvsgj-reflector

Conversation

@dkarrasch

Copy link
Copy Markdown
Member

Follow-up to #1734. The left-applying reflectorApply!(x, τ, A) used dot and axpy!, so for arrays of arrays dot recursed into the elements and the packed QR solves failed for vector-valued right-hand sides. The computation is now carried out by one of two kernels:

  • reflectorApplyNumeric!: the previous dot/axpy! implementation, used for numeric x and A;
  • reflectorApplyLoop!: explicit loops that only multiply the elements of A by the entries of x and 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 with fill!, since broadcasting an array-valued zero recurses into it as well. Numeric code paths are unchanged (timings of qrfactUnblocked! and _lqmul! on master and this branch agree within noise).

Tests: both kernels agree with each other and with the definition for Float64, ComplexF64 and Quaternion elements; a matrix of SizedArrays is reflected componentwise; the @test_broken from #1734 is replaced by solves and Q/Q' multiplications with SizedArray right-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

`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

codecov Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.10%. Comparing base (a75ca9b) to head (092f1c1).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dkarrasch dkarrasch mentioned this pull request Oct 4, 2026
1 of 2 tasks
Comment thread src/generic.jl Outdated
Comment thread src/generic.jl Outdated
Comment thread src/generic.jl Outdated
Co-authored-by: Daniel Karrasch <daniel.karrasch@posteo.de>
@dkarrasch
dkarrasch merged commit 2219e58 into master Oct 4, 2026
11 checks passed
@dkarrasch
dkarrasch deleted the claude/elegant-ramanujan-5hvsgj-reflector branch October 4, 2026 17:49
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants