Skip to content

Commit fd220fa

Browse files
jishnubdevmotion
jishnub
and
devmotion
authored
Typed Matrix constructor and more stable tests (#199)
* Typed Matrix constructor and more stable tests * Fix compat bounds * Allow BandedMatrices v0.17 * Remove untyped Matrix method * use `Symmetric` instead of `Hermitian` Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Inferred test 1 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * use `Symmetric` instead of `Hermitian` 2 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * use `Symmetric` instead of `Hermitian` 3 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * use `Symmetric` instead of `Hermitian` 4 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * use `Symmetric` instead of `Hermitian` 5 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Inferred test 2 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * use `Symmetric` instead of `Hermitian` 5 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Inferred test 3 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Drop BandedMatrices v0.17 * Test for Matrix{eltype} 1 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Test for Matrix{eltype} 2 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Inferred test 4 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Test for Matrix{eltype} 3 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * use `Symmetric` instead of `Hermitian` 6 Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Restore linear indexing methods --------- Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
1 parent cd28425 commit fd220fa

File tree

7 files changed

+42
-21
lines changed

7 files changed

+42
-21
lines changed

‎Project.toml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,17 @@ SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf"
88
SuiteSparse = "4607b0f0-06f3-5cda-b6b1-a6196a1729e9"
99

1010
[compat]
11+
BandedMatrices = "0.15, 1"
12+
Random = "<0.0.1, 1"
13+
StaticArrays = "1"
14+
Test = "<0.0.1, 1"
1115
julia = "1"
1216

1317
[extras]
1418
BandedMatrices = "aae01518-5342-5314-be14-df237901396f"
19+
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
1520
StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
1621
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
1722

1823
[targets]
19-
test = ["BandedMatrices", "StaticArrays", "Test"]
24+
test = ["BandedMatrices", "StaticArrays", "Random", "Test"]

‎src/pdiagmat.jl

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ Base.convert(::Type{AbstractPDMat{T}}, a::PDiagMat) where {T<:Real} = convert(PD
2828
### Basics
2929

3030
Base.size(a::PDiagMat) = (a.dim, a.dim)
31-
Base.Matrix(a::PDiagMat) = Matrix(Diagonal(a.diag))
31+
Base.Matrix{T}(a::PDiagMat) where {T} = Matrix{T}(Diagonal(a.diag))
3232
LinearAlgebra.diag(a::PDiagMat) = copy(a.diag)
3333
LinearAlgebra.cholesky(a::PDiagMat) = Cholesky(Diagonal(map(sqrt, a.diag)), 'U', 0)
3434

@@ -37,11 +37,7 @@ Base.broadcastable(a::PDiagMat) = Base.broadcastable(Diagonal(a.diag))
3737

3838
### Inheriting from AbstractMatrix
3939

40-
function Base.getindex(a::PDiagMat, i::Integer)
41-
ncol, nrow = fldmod1(i, a.dim)
42-
ncol == nrow ? a.diag[nrow] : zero(eltype(a))
43-
end
44-
Base.getindex(a::PDiagMat{T},i::Integer,j::Integer) where {T} = i == j ? a.diag[i] : zero(T)
40+
Base.@propagate_inbounds Base.getindex(a::PDiagMat{T}, i::Int, j::Int) where {T} = i == j ? a.diag[i] : zero(T)
4541

4642
### Arithmetics
4743

‎src/pdmat.jl

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ Base.convert(::Type{AbstractPDMat{T}}, a::PDMat) where {T<:Real} = convert(PDMat
4242
### Basics
4343

4444
Base.size(a::PDMat) = (a.dim, a.dim)
45-
Base.Matrix(a::PDMat) = Matrix(a.mat)
45+
Base.Matrix{T}(a::PDMat) where {T} = Matrix{T}(a.mat)
4646
LinearAlgebra.diag(a::PDMat) = diag(a.mat)
4747
LinearAlgebra.cholesky(a::PDMat) = a.chol
4848

@@ -51,8 +51,11 @@ Base.broadcastable(a::PDMat) = Base.broadcastable(a.mat)
5151

5252
### Inheriting from AbstractMatrix
5353

54-
Base.getindex(a::PDMat, i::Int) = getindex(a.mat, i)
55-
Base.getindex(a::PDMat, I::Vararg{Int, N}) where {N} = getindex(a.mat, I...)
54+
Base.IndexStyle(::Type{PDMat{T,S}}) where {T,S} = Base.IndexStyle(S)
55+
# Linear Indexing
56+
Base.@propagate_inbounds Base.getindex(a::PDMat, i::Int) = getindex(a.mat, i)
57+
# Cartesian Indexing
58+
Base.@propagate_inbounds Base.getindex(a::PDMat, I::Vararg{Int, 2}) = getindex(a.mat, I...)
5659

5760
### Arithmetics
5861

‎src/pdsparsemat.jl

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,17 @@ Base.convert(::Type{AbstractPDMat{T}}, a::PDSparseMat) where {T<:Real} = convert
4545
### Basics
4646

4747
Base.size(a::PDSparseMat) = (a.dim, a.dim)
48-
Base.Matrix(a::PDSparseMat) = Matrix(a.mat)
48+
Base.Matrix{T}(a::PDSparseMat) where {T} = Matrix{T}(a.mat)
4949
LinearAlgebra.diag(a::PDSparseMat) = diag(a.mat)
5050
LinearAlgebra.cholesky(a::PDSparseMat) = a.chol
5151

5252
### Inheriting from AbstractMatrix
5353

54-
Base.getindex(a::PDSparseMat,i::Integer) = getindex(a.mat, i)
55-
Base.getindex(a::PDSparseMat,I::Vararg{Int, N}) where {N} = getindex(a.mat, I...)
54+
Base.IndexStyle(::Type{PDSparseMat{T,S}}) where {T,S} = IndexStyle(S)
55+
# Linear Indexing
56+
Base.@propagate_inbounds Base.getindex(a::PDSparseMat, i::Int) = getindex(a.mat, i)
57+
# Cartesian Indexing
58+
Base.@propagate_inbounds Base.getindex(a::PDSparseMat, I::Vararg{Int, 2}) = getindex(a.mat, I...)
5659

5760
### Arithmetics
5861

‎test/pdmtypes.jl

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,19 +166,28 @@ using Test
166166

167167
@testset "AbstractPDMat constructors (#136)" begin
168168
x = rand(10, 10)
169-
A = x' * x + I
169+
A = Array(Symmetric(x' * x + I))
170170

171171
M = @inferred AbstractPDMat(A)
172172
@test M isa PDMat
173173
@test Matrix(M) ≈ A
174+
Mat32 = @inferred Matrix{Float32}(M)
175+
@test eltype(Mat32) == Float32
176+
@test Mat32 ≈ Float32.(A)
174177

175178
M = @inferred AbstractPDMat(cholesky(A))
176179
@test M isa PDMat
177180
@test Matrix(M) ≈ A
181+
Mat32 = @inferred Matrix{Float32}(M)
182+
@test Mat32 isa Matrix{Float32}
183+
@test Mat32 ≈ Float32.(A)
178184

179185
M = @inferred AbstractPDMat(Diagonal(A))
180186
@test M isa PDiagMat
181187
@test Matrix(M) ≈ Diagonal(A)
188+
Mat32 = @inferred Matrix{Float32}(M)
189+
@test Mat32 isa Matrix{Float32}
190+
@test Mat32 ≈ Float32.(Diagonal(A))
182191

183192
M = @inferred AbstractPDMat(Symmetric(Diagonal(A)))
184193
@test M isa PDiagMat
@@ -191,6 +200,9 @@ using Test
191200
M = @inferred AbstractPDMat(sparse(A))
192201
@test M isa PDSparseMat
193202
@test Matrix(M) ≈ A
203+
Mat32 = @inferred Matrix{Float32}(M)
204+
@test Mat32 isa Matrix{Float32}
205+
@test Mat32 ≈ Float32.(A)
194206

195207
if VERSION < v"1.6"
196208
# inference fails e.g. on Julia 1.0
@@ -205,7 +217,7 @@ using Test
205217
@testset "properties and fields" begin
206218
for dim in (1, 5, 10)
207219
x = rand(dim, dim)
208-
M = PDMat(x' * x + I)
220+
M = PDMat(Array(Symmetric(x' * x + I)))
209221
@test fieldnames(typeof(M)) == (:mat, :chol)
210222
@test propertynames(M) == (fieldnames(typeof(M))..., :dim)
211223
@test getproperty(M, :dim) === dim
@@ -230,7 +242,7 @@ using Test
230242

231243
if HAVE_CHOLMOD
232244
x = sprand(dim, dim, 0.2)
233-
M = PDSparseMat(x' * x + I)
245+
M = PDSparseMat(sparse(Symmetric(x' * x + I)))
234246
@test fieldnames(typeof(M)) == (:mat, :chol)
235247
@test propertynames(M) == (fieldnames(typeof(M))..., :dim)
236248
@test getproperty(M, :dim) === dim
@@ -243,14 +255,14 @@ using Test
243255

244256
@testset "Incorrect dimensions" begin
245257
x = rand(10, 10)
246-
A = x * x' + I
258+
A = Array(Symmetric(x * x' + I))
247259
C = cholesky(A)
248260
@test_throws DimensionMismatch PDMat(A[:, 1:(end - 1)], C)
249261
@test_throws DimensionMismatch PDMat(A[1:(end - 1), 1:(end - 1)], C)
250262

251263
if HAVE_CHOLMOD
252264
x = sprand(10, 10, 0.2)
253-
A = x * x' + I
265+
A = sparse(Symmetric(x * x' + I))
254266
C = cholesky(A)
255267
@test_throws DimensionMismatch PDSparseMat(A[:, 1:(end - 1)], C)
256268
@test_throws DimensionMismatch PDSparseMat(A[1:(end - 1), 1:(end - 1)], C)
@@ -261,7 +273,7 @@ using Test
261273
# This falls back to the generic method in Julia based on broadcasting
262274
dim = 4
263275
x = rand(dim, dim)
264-
A = PDMat(x' * x + I)
276+
A = PDMat(Array(Symmetric(x' * x + I)))
265277
@test Base.broadcastable(A) == A.mat
266278

267279
B = PDiagMat(rand(dim))

‎test/specialarrays.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ using StaticArrays
44
@testset "Special matrix types" begin
55
@testset "StaticArrays" begin
66
# Full matrix
7-
S = (x -> x * x' + I)(@SMatrix(randn(4, 7)))
7+
S = (x -> SMatrix{4,4}(Symmetric(x * x' + I)))(@SMatrix(randn(4, 7)))
88
PDS = PDMat(S)
99
@test PDS isa PDMat{Float64, <:SMatrix{4, 4, Float64}}
1010
@test isbits(PDS)

‎test/testutils.jl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
# the implementation of a subtype of AbstractPDMat
55
#
66

7-
using PDMats, SuiteSparse, Test
7+
using PDMats, SuiteSparse, Test, Random
8+
9+
Random.seed!(10)
810

911
const HAVE_CHOLMOD = isdefined(SuiteSparse, :CHOLMOD)
1012
const PDMatType = HAVE_CHOLMOD ? Union{PDMat, PDSparseMat, PDiagMat} : Union{PDMat, PDiagMat}

0 commit comments

Comments
 (0)