Skip to content

Commit e6b2f0e

Browse files
dkarraschclaude
andauthored
Defer HessenbergQ multiplication to LinearAlgebra where available (#185)
* Defer HessenbergQ multiplication to LinearAlgebra where available LinearAlgebra now ships generic `lmul!`/`rmul!` methods for `HessenbergQ` with the same signatures as the ones defined here. Redefining them overwrites the upstream methods, which is an error during precompilation, so only define the local fallbacks when LinearAlgebra lacks them. The `HessenbergQ`s built in `BidiagonalFactorization.getproperty` store their reflectors below the subdiagonal, which is the `'L'` layout that `hessenberg` produces; mark them as such instead of `'U'`. Also pass the matching `τ` type parameter for `leftQ`. Extend the HessenbergQ test to both factors, both bidiagonal orientations, real and complex BigFloat, all four multiplication directions, and the reconstruction of the original matrix. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vv9VYe4nM23v8ruYwk9AQx * bump version --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 658cf62 commit e6b2f0e

3 files changed

Lines changed: 32 additions & 10 deletions

File tree

‎Project.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
name = "GenericLinearAlgebra"
22
uuid = "14197337-ba66-59df-a3e3-ca00e7dcff7a"
3-
version = "0.4.1"
3+
version = "0.4.2"
44

55
[deps]
66
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"

‎src/svd.jl‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ using LinearAlgebra
22

33
import LinearAlgebra: mul!, rmul!
44

5-
AdjointQtype = isdefined(LinearAlgebra, :AdjointQ) ? LinearAlgebra.AdjointQ : Adjoint
6-
75
lmul!(G::LinearAlgebra.Givens, ::Nothing) = nothing
86
rmul!(::Nothing, G::LinearAlgebra.Givens) = nothing
97

@@ -399,7 +397,7 @@ function Base.getproperty(F::BidiagonalFactorization, s::Symbol)
399397
typeof(τr),
400398
false,
401399
}(
402-
'U',
400+
'L',
403401
factors,
404402
τr,
405403
)
@@ -412,10 +410,10 @@ function Base.getproperty(F::BidiagonalFactorization, s::Symbol)
412410
return LinearAlgebra.HessenbergQ{
413411
eltype(factors),
414412
typeof(factors),
415-
typeof(τr),
413+
typeof(τl),
416414
false,
417415
}(
418-
'U',
416+
'L',
419417
factors,
420418
τl,
421419
)
@@ -429,7 +427,13 @@ function Base.getproperty(F::BidiagonalFactorization, s::Symbol)
429427
end
430428
end
431429

432-
# For now, we define a generic lmul! and rmul! for HessenbergQ here
430+
# LinearAlgebra's generic `HessenbergQ` multiplication (available in versions that ship it)
431+
# handles non-BLAS element types and the `uplo` field, so these fallbacks are only needed for
432+
# older versions. Defining them unconditionally would overwrite the upstream methods, which is
433+
# an error during precompilation.
434+
@static if !hasmethod(lmul!, Tuple{LinearAlgebra.HessenbergQ,AbstractVecOrMat})
435+
AdjointQtype = isdefined(LinearAlgebra, :AdjointQ) ? LinearAlgebra.AdjointQ : Adjoint
436+
433437
function lmul!(Q::LinearAlgebra.HessenbergQ, B::AbstractVecOrMat)
434438

435439
m, n = size(B, 1), size(B, 2)
@@ -530,6 +534,7 @@ function rmul!(A::AbstractMatrix, adjQ::AdjointQtype{<:Any,<:LinearAlgebra.Hesse
530534

531535
return A
532536
end
537+
end
533538

534539
# FIXME! Commented out for now because we currently don't produce
535540
# any LQPackedQ matrices.

‎test/svd.jl‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,10 +249,27 @@ using Test, GenericLinearAlgebra, LinearAlgebra, Quaternions, DoubleFloats
249249
@test diag(F.U' * B * F.Vt') ≈ F.S rtol = 5e-15
250250
end
251251

252-
@testset "Generic HessenbergQ multiplication" begin
253-
A = big.(randn(10, 10))
252+
# the tall case stores the right reflectors in a HessenbergQ, the wide case the left ones
253+
@testset "Generic HessenbergQ multiplication: big($T), ($m, $n)" for T in (Float64, ComplexF64),
254+
(m, n) in ((10, 10), (10, 6), (6, 10))
255+
256+
A = big.(randn(T, m, n))
257+
TB = eltype(A)
254258
BF = GenericLinearAlgebra.bidiagonalize!(copy(A))
255-
@test (BF.rightQ' * Matrix(I, size(A)...)) * BF.rightQ ≈ I
259+
for Q in (BF.leftQ, BF.rightQ)
260+
k = size(Q, 1)
261+
QI = Q * Matrix{TB}(I, k, k)
262+
@test Q' * QI ≈ I
263+
B = big.(randn(T, k, 3))
264+
C = big.(randn(T, 3, k))
265+
@test lmul!(Q, copy(B)) ≈ QI * B
266+
@test lmul!(Q', copy(B)) ≈ QI' * B
267+
@test rmul!(copy(C), Q) ≈ C * QI
268+
@test rmul!(copy(C), Q') ≈ C * QI'
269+
end
270+
Bd = zeros(TB, m, n)
271+
Bd[1:min(m, n), 1:min(m, n)] = BF.bidiagonal
272+
@test BF.leftQ * Bd * BF.rightQ' ≈ A
256273
end
257274

258275
@testset "Issue 119" begin

0 commit comments

Comments
 (0)