Skip to content

Commit

Permalink
Don't use copy_oftype in \(QRX, B) to make sure that solution array i…
Browse files Browse the repository at this point in the history
…s mutable (#24110)

* Don't use copy_oftype in \(QRX, B) to make sure that solution array
is mutable. Fixes #24107

* Always return B in A_ldiv_B!(QRPivoted, B)
  • Loading branch information
andreasnoack committed Oct 15, 2017
1 parent 9eebbf9 commit f9c662d
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 21 deletions.
34 changes: 13 additions & 21 deletions base/linalg/qr.jl
Expand Up @@ -775,11 +775,12 @@ function A_ldiv_B!(A::QRPivoted{T}, B::StridedMatrix{T}, rcond::Real) where T<:B
nr = min(mA,nA)
nrhs = size(B, 2)
if nr == 0
return zeros(T, 0, nrhs), 0
return B, 0
end
ar = abs(A.factors[1])
if ar == 0
return zeros(T, nA, nrhs), 0
B[1:nA, :] = 0
return B, 0
end
rnk = 1
xmin = ones(T, 1)
Expand Down Expand Up @@ -873,23 +874,8 @@ _cut_B(x::AbstractVector, r::UnitRange) = length(x) > length(r) ? x[r] : x
_cut_B(X::AbstractMatrix, r::UnitRange) = size(X, 1) > length(r) ? X[r,:] : X

## append right hand side with zeros if necessary
function _append_zeros(b::AbstractVector, T::Type, n)
if n > length(b)
x = zeros(T, n)
return copy!(x, b)
else
return copy_oftype(b, T)
end
end
function _append_zeros(B::AbstractMatrix, T::Type, n)
if n > size(B, 1)
X = zeros(T, (n, size(B, 2)))
X[1:size(B,1), :] = B
return X
else
return copy_oftype(B, T)
end
end
_zeros(::Type{T}, b::AbstractVector, n::Integer) where {T} = zeros(T, max(length(b), n))
_zeros(::Type{T}, B::AbstractMatrix, n::Integer) where {T} = zeros(T, max(size(B, 1), n), size(B, 2))

function (\)(A::Union{QR{TA},QRCompactWY{TA},QRPivoted{TA}}, B::AbstractVecOrMat{TB}) where {TA,TB}
S = promote_type(TA,TB)
Expand All @@ -898,7 +884,10 @@ function (\)(A::Union{QR{TA},QRCompactWY{TA},QRPivoted{TA}}, B::AbstractVecOrMat

AA = convert(Factorization{S}, A)

X = A_ldiv_B!(AA, _append_zeros(B, S, n))
X = _zeros(S, B, n)
X[1:size(B, 1), :] = B

A_ldiv_B!(AA, X)

return _cut_B(X, 1:n)
end
Expand All @@ -920,7 +909,10 @@ function (\)(A::Union{QR{T},QRCompactWY{T},QRPivoted{T}}, BIn::VecOrMat{Complex{
# |x4|y4|
B = reshape(transpose(reinterpret(T, reshape(BIn, (1, length(BIn))))), size(BIn, 1), 2*size(BIn, 2))

X = A_ldiv_B!(A, _append_zeros(B, T, n))
X = _zeros(T, B, n)
X[1:size(B, 1), :] = B

A_ldiv_B!(A, X)

# |z1|z3| reinterpret |x1|x2|x3|x4| transpose |x1|y1| reshape |x1|y1|x3|y3|
# |z2|z4| <- |y1|y2|y3|y4| <- |x2|y2| <- |x2|y2|x4|y4|
Expand Down
5 changes: 5 additions & 0 deletions test/linalg/qr.jl
Expand Up @@ -197,3 +197,8 @@ end
@test A \ B == zeros(2, 1)
@test qrfact(A, Val(true)) \ B == zeros(2, 1)
end

@testset "Issue 24107" begin
A = rand(200,2)
@test A \ linspace(0,1,200) == A \ collect(linspace(0,1,200))
end

0 comments on commit f9c662d

Please sign in to comment.