Skip to content

Improve linalg error messages and coverage reports, round 2 #12434

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Aug 11, 2015
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions base/linalg/bidiag.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type Bidiagonal{T} <: AbstractMatrix{T}
if length(ev)==length(dv)-1
new(dv, ev, isupper)
else
throw(DimensionMismatch("Length of diagonal vector is $(length(dv)), length of off-diagonal vector is $(length(ev))"))
throw(DimensionMismatch("length of diagonal vector is $(length(dv)), length of off-diagonal vector is $(length(ev))"))
end
end
end
Expand All @@ -35,7 +35,9 @@ end
Bidiagonal(A::AbstractMatrix, isupper::Bool)=Bidiagonal(diag(A), diag(A, isupper?1:-1), isupper)

function getindex{T}(A::Bidiagonal{T}, i::Integer, j::Integer)
(1<=i<=size(A,2) && 1<=j<=size(A,2)) || throw(BoundsError())
if !(1<=i<=size(A,2) && 1<=j<=size(A,2))
throw(BoundsError(A,(i,j)))
end
i == j ? A.dv[i] : (A.isupper && (i == j-1)) || (!A.isupper && (i == j+1)) ? A.ev[min(i,j)] : zero(T)
end

Expand Down Expand Up @@ -81,7 +83,15 @@ function show(io::IO, M::Bidiagonal)
end

size(M::Bidiagonal) = (length(M.dv), length(M.dv))
size(M::Bidiagonal, d::Integer) = d<1 ? throw(ArgumentError("dimension must be ≥ 1, got $d")) : (d<=2 ? length(M.dv) : 1)
function size(M::Bidiagonal, d::Integer)
if d < 1
throw(ArgumentError("dimension must be ≥ 1, got $d"))
elseif d <= 2
return length(M.dv)
else
return 1
end
end

#Elementary operations
for func in (:conj, :copy, :round, :trunc, :floor, :ceil)
Expand Down Expand Up @@ -184,7 +194,7 @@ At_ldiv_B(A::Union{Bidiagonal, AbstractTriangular}, B::AbstractMatrix) = At_ldiv
function naivesub!{T}(A::Bidiagonal{T}, b::AbstractVector, x::AbstractVector = b)
N = size(A, 2)
if N != length(b) || N != length(x)
throw(DimensionMismatch())
throw(DimensionMismatch("second dimension of A, $N, does not match one of the lengths of x, $(length(x)), or b, $(length(b))"))
end
if !A.isupper #do forward substitution
for j = 1:N
Expand Down
8 changes: 5 additions & 3 deletions base/linalg/dense.jl
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ vecnorm2{T<:BlasFloat}(x::Union{Array{T},StridedVector{T}}) =
function triu!(M::AbstractMatrix, k::Integer)
m, n = size(M)
if (k > 0 && k > n) || (k < 0 && -k > m)
throw(BoundsError())
throw(ArgumentError("requested diagonal, $k, out of bounds in matrix of size ($m,$n)"))
end
idx = 1
for j = 0:n-1
Expand All @@ -89,7 +89,7 @@ triu(M::Matrix, k::Integer) = triu!(copy(M), k)
function tril!(M::AbstractMatrix, k::Integer)
m, n = size(M)
if (k > 0 && k > n) || (k < 0 && -k > m)
throw(BoundsError())
throw(ArgumentError("requested diagonal, $k, out of bounds in matrix of size ($m,$n)"))
end
idx = 1
for j = 0:n-1
Expand Down Expand Up @@ -122,7 +122,9 @@ function gradient(F::Vector, h::Vector)
end

function diagind(m::Integer, n::Integer, k::Integer=0)
-m <= k <= n || throw(BoundsError())
if !(-m <= k <= n)
throw(ArgumentError("requested diagonal, $k, out of bounds in matrix of size ($m,$n)"))
end
k <= 0 ? range(1-k, m+1, min(m+k, n)) : range(k*m+1, m+1, min(m, n-k))
end

Expand Down
4 changes: 2 additions & 2 deletions base/linalg/diagonal.jl
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ setindex!(D::Diagonal, v, i::Int, j::Int) = (checkbounds(D, i, j); unsafe_setind
function unsafe_setindex!(D::Diagonal, v, i::Int, j::Int)
if i == j
unsafe_setindex!(D.diag, v, i)
else
v == 0 || throw(ArgumentError("cannot set an off-diagonal index ($i, $j) to a nonzero value ($v)"))
elseif v != 0
throw(ArgumentError("cannot set an off-diagonal index ($i, $j) to a nonzero value ($v)"))
end
D
end
Expand Down
10 changes: 6 additions & 4 deletions base/linalg/generic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ function generic_scale!(X::AbstractArray, s::Number)
end

function generic_scale!(C::AbstractArray, X::AbstractArray, s::Number)
length(C) == length(X) || throw(DimensionMismatch("first array argument must be the same length as the second array argument"))
if length(C) != length(X)
throw(DimensionMismatch("first array has length $(length(C)) which does not match the length of the second, $(length(X))."))
end
for i = 1:length(X)
@inbounds C[i] = X[i]*s
end
Expand Down Expand Up @@ -58,7 +60,7 @@ end
gradient(F::AbstractVector) = gradient(F, [1:length(F);])
gradient(F::AbstractVector, h::Real) = gradient(F, [h*(1:length(F));])

diag(A::AbstractVector) = error("use diagm instead of diag to construct a diagonal matrix")
diag(A::AbstractVector) = throw(ArgumentError("use diagm instead of diag to construct a diagonal matrix"))

#diagm{T}(v::AbstractVecOrMat{T})

Expand Down Expand Up @@ -233,7 +235,7 @@ norm(x::Number, p::Real=2) =
function vecdot(x::AbstractVector, y::AbstractVector)
lx = length(x)
if lx != length(y)
throw(DimensionMismatch("Vector x has length $lx, but vector y has length $(length(y))"))
throw(DimensionMismatch("vector x has length $lx, but vector y has length $(length(y))"))
end
if lx == 0
return dot(zero(eltype(x)), zero(eltype(y)))
Expand Down Expand Up @@ -301,7 +303,7 @@ trace(x::Number) = x

#det(a::AbstractMatrix)

inv(a::StridedMatrix) = error("argument must be a square matrix")
inv(a::StridedMatrix) = throw(ArgumentError("argument must be a square matrix"))
function inv{T}(A::AbstractMatrix{T})
S = typeof(zero(T)/one(T))
A_ldiv_B!(factorize(convert(AbstractMatrix{S}, A)), eye(S, chksquare(A)))
Expand Down
34 changes: 22 additions & 12 deletions base/linalg/matmul.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ arithtype(::Type{Bool}) = Int
function scale!(C::AbstractMatrix, A::AbstractMatrix, b::AbstractVector)
m, n = size(A)
p, q = size(C)
if n != length(b) || p != m || q != n
throw(DimensionMismatch())
if size(A) != size(C)
throw(DimensionMismatch("size of A, $(size(A)), does not match size of C, $(size(C))"))
end
if n != length(b)
throw(DimensionMismatch("second dimension of A, $n, does not match length of b, $(length(b))"))
end
@inbounds for j = 1:n
bj = b[j]
Expand All @@ -24,8 +27,11 @@ end
function scale!(C::AbstractMatrix, b::AbstractVector, A::AbstractMatrix)
m, n = size(A)
p, q = size(C)
if m != length(b) || p != m || q != n
throw(DimensionMismatch())
if size(A) != size(C)
throw(DimensionMismatch("size of A, $(size(A)), does not match size of C, $(size(C))"))
end
if m != length(b)
throw(DimensionMismatch("first dimension of A, $m, does not match length of b, $(length(b))"))
end
@inbounds for j = 1:n, i = 1:m
C[i,j] = A[i,j]*b[i]
Expand All @@ -40,14 +46,18 @@ scale(b::Vector, A::Matrix) = scale!(similar(b, promote_type(eltype(A),eltype(b)
vecdot{T<:BlasReal}(x::Union{DenseArray{T},StridedVector{T}}, y::Union{DenseArray{T},StridedVector{T}}) = BLAS.dot(x, y)
vecdot{T<:BlasComplex}(x::Union{DenseArray{T},StridedVector{T}}, y::Union{DenseArray{T},StridedVector{T}}) = BLAS.dotc(x, y)
function dot{T<:BlasReal, TI<:Integer}(x::Vector{T}, rx::Union{UnitRange{TI},Range{TI}}, y::Vector{T}, ry::Union{UnitRange{TI},Range{TI}})
length(rx)==length(ry) || throw(DimensionMismatch())
if length(rx) != length(ry)
throw(DimensionMismatch("length of rx, $(length(rx)), does not equal length of ry, $(length(ry))"))
end
if minimum(rx) < 1 || maximum(rx) > length(x) || minimum(ry) < 1 || maximum(ry) > length(y)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be split out, and have information in the BoundsError(s)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one still has no information in the BoundsError

throw(BoundsError())
end
BLAS.dot(length(rx), pointer(x)+(first(rx)-1)*sizeof(T), step(rx), pointer(y)+(first(ry)-1)*sizeof(T), step(ry))
end
function dot{T<:BlasComplex, TI<:Integer}(x::Vector{T}, rx::Union{UnitRange{TI},Range{TI}}, y::Vector{T}, ry::Union{UnitRange{TI},Range{TI}})
length(rx)==length(ry) || throw(DimensionMismatch())
if length(rx) != length(ry)
throw(DimensionMismatch("length of rx, $(length(rx)), does not equal length of ry, $(length(ry))"))
end
if minimum(rx) < 1 || maximum(rx) > length(x) || minimum(ry) < 1 || maximum(ry) > length(y)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, no info in BoundsError

throw(BoundsError())
end
Expand Down Expand Up @@ -202,10 +212,10 @@ end
function gemv!{T<:BlasFloat}(y::StridedVector{T}, tA::Char, A::StridedVecOrMat{T}, x::StridedVector{T})
mA, nA = lapack_size(tA, A)
if nA != length(x)
throw(DimensionMismatch())
throw(DimensionMismatch("second dimension of A, $nA, does not match length of x, $(length(x))"))
end
if mA != length(y)
throw(DimensionMismatch())
throw(DimensionMismatch("first dimension of A, $mA, does not match length of y, $(length(y))"))
end
if mA == 0
return y
Expand Down Expand Up @@ -348,10 +358,10 @@ function generic_matvecmul!{T,S,R}(C::AbstractVector{R}, tA, A::AbstractVecOrMat
mB = length(B)
mA, nA = lapack_size(tA, A)
if mB != nA
throw(DimensionMismatch("Matrix A has dimensions ($mA,$nA), vector B has length $mB"))
throw(DimensionMismatch("matrix A has dimensions ($mA,$nA), vector B has length $mB"))
end
if mA != length(C)
throw(DimensionMismatch("Result C has length $(length(C)), needs length $mA"))
throw(DimensionMismatch("result C has length $(length(C)), needs length $mA"))
end
z = zero(R)

Expand Down Expand Up @@ -404,10 +414,10 @@ function generic_matmatmul!{T,S,R}(C::AbstractVecOrMat{R}, tA, tB, A::AbstractVe
mA, nA = lapack_size(tA, A)
mB, nB = lapack_size(tB, B)
if mB != nA
throw(DimensionMismatch("Matrix A has dimensions ($mA, $nB), matrix B has dimensions ($mB, $nB)"))
throw(DimensionMismatch("matrix A has dimensions ($mA, $nB), matrix B has dimensions ($mB, $nB)"))
end
if size(C,1) != mA || size(C,2) != nB
throw(DimensionMismatch("Result C has dimensions $(size(C)), needs ($mA, $nB)"))
throw(DimensionMismatch("result C has dimensions $(size(C)), needs ($mA, $nB)"))
end

if mA == nA == nB == 2
Expand Down
38 changes: 27 additions & 11 deletions base/linalg/special.jl
Original file line number Diff line number Diff line change
Expand Up @@ -13,51 +13,67 @@ convert(::Type{UpperTriangular}, A::Bidiagonal) = A.isupper ? UpperTriangular(fu
convert(::Type{Matrix}, D::Diagonal) = diagm(D.diag)

function convert(::Type{UnitUpperTriangular}, A::Diagonal)
all(A.diag .== one(eltype(A))) || throw(ArgumentError("Matrix cannot be represented as UnitUpperTriangular"))
if !all(A.diag .== one(eltype(A)))
throw(ArgumentError("matrix cannot be represented as UnitUpperTriangular"))
end
UnitUpperTriangular(full(A))
end

function convert(::Type{UnitLowerTriangular}, A::Diagonal)
all(A.diag .== one(eltype(A))) || throw(ArgumentError("Matrix cannot be represented as UnitLowerTriangular"))
if !all(A.diag .== one(eltype(A)))
throw(ArgumentError("matrix cannot be represented as UnitLowerTriangular"))
end
UnitLowerTriangular(full(A))
end

function convert(::Type{Diagonal}, A::Union{Bidiagonal, SymTridiagonal})
all(A.ev .== 0) || throw(ArgumentError("Matrix cannot be represented as Diagonal"))
if !all(A.ev .== 0)
throw(ArgumentError("matrix cannot be represented as Diagonal"))
end
Diagonal(A.dv)
end

function convert(::Type{SymTridiagonal}, A::Bidiagonal)
all(A.ev .== 0) || throw(ArgumentError("Matrix cannot be represented as SymTridiagonal"))
if !all(A.ev .== 0)
throw(ArgumentError("matrix cannot be represented as SymTridiagonal"))
end
SymTridiagonal(A.dv, A.ev)
end

convert{T}(::Type{Tridiagonal}, A::Bidiagonal{T})=Tridiagonal(A.isupper?zeros(T, size(A.dv,1)-1):A.ev, A.dv, A.isupper?A.ev:zeros(T, size(A.dv,1)-1))

function convert(::Type{Bidiagonal}, A::SymTridiagonal)
all(A.ev .== 0) || throw(ArgumentError("Matrix cannot be represented as Bidiagonal"))
if !all(A.ev .== 0)
throw(ArgumentError("matrix cannot be represented as Bidiagonal"))
end
Bidiagonal(A.dv, A.ev, true)
end

function convert(::Type{Diagonal}, A::Tridiagonal)
all(A.dl .== 0) && all(A.du .== 0) || throw(ArgumentError("Matrix cannot be represented as Diagonal"))
if !(all(A.dl .== 0) && all(A.du .== 0))
throw(ArgumentError("matrix cannot be represented as Diagonal"))
end
Diagonal(A.d)
end

function convert(::Type{Bidiagonal}, A::Tridiagonal)
if all(A.dl .== 0) return Bidiagonal(A.d, A.du, true)
elseif all(A.du .== 0) return Bidiagonal(A.d, A.dl, false)
else throw(ArgumentError("Matrix cannot be represented as Bidiagonal"))
else throw(ArgumentError("matrix cannot be represented as Bidiagonal"))
end
end

function convert(::Type{SymTridiagonal}, A::Tridiagonal)
all(A.dl .== A.du) || throw(ArgumentError("Matrix cannot be represented as SymTridiagonal"))
if !all(A.dl .== A.du)
throw(ArgumentError("matrix cannot be represented as SymTridiagonal"))
end
SymTridiagonal(A.d, A.dl)
end

function convert(::Type{Diagonal}, A::AbstractTriangular)
full(A) == diagm(diag(A)) || throw(ArgumentError("Matrix cannot be represented as Diagonal"))
if full(A) != diagm(diag(A))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreasnoack it seems terribly inefficient to do a full conversion here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. That is bananas. Allocating three arrays for an operation that doesn't require a single is not efficient. I'll prepare a fix when this is merged.

throw(ArgumentError("matrix cannot be represented as Diagonal"))
end
Diagonal(diag(A))
end

Expand All @@ -68,7 +84,7 @@ function convert(::Type{Bidiagonal}, A::AbstractTriangular)
elseif fA == diagm(diag(A)) + diagm(diag(fA, -1), -1)
return Bidiagonal(diag(A), diag(fA,-1), false)
else
throw(ArgumentError("Matrix cannot be represented as Bidiagonal"))
throw(ArgumentError("matrix cannot be represented as Bidiagonal"))
end
end

Expand All @@ -79,7 +95,7 @@ function convert(::Type{Tridiagonal}, A::AbstractTriangular)
if fA == diagm(diag(A)) + diagm(diag(fA, 1), 1) + diagm(diag(fA, -1), -1)
return Tridiagonal(diag(fA, -1), diag(A), diag(fA,1))
else
throw(ArgumentError("Matrix cannot be represented as Tridiagonal"))
throw(ArgumentError("matrix cannot be represented as Tridiagonal"))
end
end

Expand Down
Loading