Skip to content

Commit b730db2

Browse files
andreasnoackandreasnoack
authored
Fix "Issue 9160" tests in cholmod.jl by removing empty loop. Update (#19585)
tests to changes in CHOLMOD module, i.e. only support SuiteSparse_long. Fixes #19583 Sort sparse matrices before constructing SparseMatrixCSC from CHOLMOD.Sparse. This issue was revealed by the fix of the "Issue 9160" tests. Add sort! method from CHOLMOD to sort CHOLMOD.Sparse matrices. Make sure that CHOLMOD.Sparse arrays get sorted indices when constructed.
1 parent 2d8f5bf commit b730db2

File tree

2 files changed

+81
-34
lines changed

2 files changed

+81
-34
lines changed

‎base/sparse/cholmod.jl

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,13 @@ function copy_dense{Tv<:VTypes}(A::Dense{Tv})
418418
d
419419
end
420420

421+
function sort!{Tv<:VTypes}(S::Sparse{Tv})
422+
@isok ccall((:cholmod_l_sort, :libcholmod), SuiteSparse_long,
423+
(Ptr{C_Sparse{Tv}}, Ptr{UInt8}),
424+
get(S.p), common())
425+
return S
426+
end
427+
421428
### cholmod_matrixops.h ###
422429
function norm_dense{Tv<:VTypes}(D::Dense{Tv}, p::Integer)
423430
s = unsafe_load(get(D.p))
@@ -851,6 +858,9 @@ end
851858
function (::Type{Sparse}){Tv<:VTypes}(m::Integer, n::Integer, colptr::Vector{SuiteSparse_long}, rowval::Vector{SuiteSparse_long}, nzval::Vector{Tv})
852859
o = Sparse(m, n, colptr, rowval, nzval, 0)
853860

861+
# sort indices
862+
sort!(o)
863+
854864
# check if array is symmetric and change stype if it is
855865
if ishermitian(o)
856866
change_stype!(o, -1)
@@ -993,21 +1003,51 @@ function convert{Tv}(::Type{SparseMatrixCSC{Tv,SuiteSparse_long}}, A::Sparse{Tv}
9931003
if s.stype != 0
9941004
throw(ArgumentError("matrix has stype != 0. Convert to matrix with stype == 0 before converting to SparseMatrixCSC"))
9951005
end
996-
return SparseMatrixCSC(s.nrow, s.ncol, increment(unsafe_wrap(Array, s.p, (s.ncol + 1,), false)), increment(unsafe_wrap(Array, s.i, (s.nzmax,), false)), copy(unsafe_wrap(Array, s.x, (s.nzmax,), false)))
1006+
1007+
B = SparseMatrixCSC(s.nrow, s.ncol,
1008+
increment(unsafe_wrap(Array, s.p, (s.ncol + 1,), false)),
1009+
increment(unsafe_wrap(Array, s.i, (s.nzmax,), false)),
1010+
copy(unsafe_wrap(Array, s.x, (s.nzmax,), false)))
1011+
1012+
if s.sorted == 0
1013+
return SparseArrays.sortSparseMatrixCSC!(B)
1014+
else
1015+
return B
1016+
end
9971017
end
9981018
function convert(::Type{Symmetric{Float64,SparseMatrixCSC{Float64,SuiteSparse_long}}}, A::Sparse{Float64})
9991019
s = unsafe_load(A.p)
10001020
if !issymmetric(A)
10011021
throw(ArgumentError("matrix is not symmetric"))
10021022
end
1003-
return Symmetric(SparseMatrixCSC(s.nrow, s.ncol, increment(unsafe_wrap(Array, s.p, (s.ncol + 1,), false)), increment(unsafe_wrap(Array, s.i, (s.nzmax,), false)), copy(unsafe_wrap(Array, s.x, (s.nzmax,), false))), s.stype > 0 ? :U : :L)
1023+
1024+
B = Symmetric(SparseMatrixCSC(s.nrow, s.ncol,
1025+
increment(unsafe_wrap(Array, s.p, (s.ncol + 1,), false)),
1026+
increment(unsafe_wrap(Array, s.i, (s.nzmax,), false)),
1027+
copy(unsafe_wrap(Array, s.x, (s.nzmax,), false))), s.stype > 0 ? :U : :L)
1028+
1029+
if s.sorted == 0
1030+
return SparseArrays.sortSparseMatrixCSC!(B.data)
1031+
else
1032+
return B
1033+
end
10041034
end
10051035
function convert{Tv<:VTypes}(::Type{Hermitian{Tv,SparseMatrixCSC{Tv,SuiteSparse_long}}}, A::Sparse{Tv})
10061036
s = unsafe_load(A.p)
10071037
if !ishermitian(A)
10081038
throw(ArgumentError("matrix is not Hermitian"))
10091039
end
1010-
return Hermitian(SparseMatrixCSC(s.nrow, s.ncol, increment(unsafe_wrap(Array, s.p, (s.ncol + 1,), false)), increment(unsafe_wrap(Array, s.i, (s.nzmax,), false)), copy(unsafe_wrap(Array, s.x, (s.nzmax,), false))), s.stype > 0 ? :U : :L)
1040+
1041+
B = Hermitian(SparseMatrixCSC(s.nrow, s.ncol,
1042+
increment(unsafe_wrap(Array, s.p, (s.ncol + 1,), false)),
1043+
increment(unsafe_wrap(Array, s.i, (s.nzmax,), false)),
1044+
copy(unsafe_wrap(Array, s.x, (s.nzmax,), false))), s.stype > 0 ? :U : :L)
1045+
1046+
if s.sorted == 0
1047+
return SparseArrays.sortSparseMatrixCSC!(B.data)
1048+
else
1049+
return B
1050+
end
10111051
end
10121052
function sparse(A::Sparse{Float64}) # Notice! Cannot be type stable because of stype
10131053
s = unsafe_load(A.p)
@@ -1571,21 +1611,16 @@ function isposdef{Tv<:VTypes}(A::SparseMatrixCSC{Tv,SuiteSparse_long})
15711611
true
15721612
end
15731613

1574-
function issymmetric(A::Sparse)
1575-
s = unsafe_load(A.p)
1576-
if s.stype != 0
1577-
return isreal(A)
1578-
end
1579-
i = symmetry(A, 1)[1]
1580-
return i == MM_SYMMETRIC || i == MM_SYMMETRIC_POSDIAG
1581-
end
1582-
15831614
function ishermitian(A::Sparse{Float64})
15841615
s = unsafe_load(A.p)
15851616
if s.stype != 0
15861617
return true
15871618
else
15881619
i = symmetry(A, 1)[1]
1620+
if i < 0
1621+
throw(CHOLMODException("negative value returned from CHOLMOD's symmetry function. This
1622+
is either because the indices are not sorted or because of a memory error"))
1623+
end
15891624
return i == MM_SYMMETRIC || i == MM_SYMMETRIC_POSDIAG
15901625
end
15911626
end
@@ -1595,6 +1630,10 @@ function ishermitian(A::Sparse{Complex{Float64}})
15951630
return true
15961631
else
15971632
i = symmetry(A, 1)[1]
1633+
if i < 0
1634+
throw(CHOLMODException("negative value returned from CHOLMOD's symmetry function. This
1635+
is either because the indices are not sorted or because of a memory error"))
1636+
end
15981637
return i == MM_HERMITIAN || i == MM_HERMITIAN_POSDIAG
15991638
end
16001639
end

‎test/sparse/cholmod.jl

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -150,35 +150,31 @@ pred = afiro'*sol
150150

151151
let # Issue 9160
152152

153-
for Ti in CHOLMOD.ITypes.types
154-
for elty in CHOLMOD.VRealTypes.types
153+
A = sprand(10, 10, 0.1)
154+
A = convert(SparseMatrixCSC{Float64,CHOLMOD.SuiteSparse_long}, A)
155+
cmA = CHOLMOD.Sparse(A)
155156

156-
A = sprand(10,10,0.1)
157-
A = convert(SparseMatrixCSC{elty,Ti},A)
158-
cmA = CHOLMOD.Sparse(A)
157+
B = sprand(10, 10, 0.1)
158+
B = convert(SparseMatrixCSC{Float64,CHOLMOD.SuiteSparse_long}, B)
159+
cmB = CHOLMOD.Sparse(B)
159160

160-
B = sprand(10,10,0.1)
161-
B = convert(SparseMatrixCSC{elty,Ti},B)
162-
cmB = CHOLMOD.Sparse(B)
161+
# Ac_mul_B
162+
@test sparse(cmA'*cmB) ≈ A'*B
163163

164-
# Ac_mul_B
165-
@test sparse(cmA'*cmB) ≈ A'*B
164+
# A_mul_Bc
165+
@test sparse(cmA*cmB') ≈ A*B'
166166

167-
# A_mul_Bc
168-
@test sparse(cmA*cmB') ≈ A*B'
167+
# A_mul_Ac
168+
@test sparse(cmA*cmA') ≈ A*A'
169169

170-
# A_mul_Ac
171-
@test sparse(cmA*cmA') ≈ A*A'
170+
# Ac_mul_A
171+
@test sparse(cmA'*cmA) ≈ A'*A
172172

173-
# Ac_mul_A
174-
@test sparse(cmA'*cmA) ≈ A'*A
173+
# A_mul_Ac for symmetric A
174+
A = 0.5*(A + A')
175+
cmA = CHOLMOD.Sparse(A)
176+
@test sparse(cmA*cmA') ≈ A*A'
175177

176-
# A_mul_Ac for symmetric A
177-
A = 0.5*(A + A')
178-
cmA = CHOLMOD.Sparse(A)
179-
@test sparse(cmA*cmA') ≈ A*A'
180-
end
181-
end
182178
end
183179

184180
# Issue #9915
@@ -687,3 +683,15 @@ let Apre = sprandn(10, 10, 0.2) - I
687683
end
688684
end
689685

686+
# Check that Symmetric{SparseMatrixCSC} can be constructed from CHOLMOD.Sparse
687+
let A = sprandn(10, 10, 0.1)
688+
B = SparseArrays.CHOLMOD.Sparse(A)
689+
C = B'B
690+
# Change internal representation to symmetric (upper/lower)
691+
o = fieldoffset(CHOLMOD.C_Sparse{eltype(C)}, find(fieldnames(CHOLMOD.C_Sparse{eltype(C)}) .== :stype)[1])
692+
for uplo in (1, -1)
693+
unsafe_store!(Ptr{Int8}(C.p), uplo, Int(o) + 1)
694+
@test convert(Symmetric{Float64,SparseMatrixCSC{Float64,Int}}, C) == Symmetric(A'A)
695+
end
696+
end
697+

0 commit comments

Comments
 (0)