From 7de0359c2548cd79f420c547d54bcab0f0f44de5 Mon Sep 17 00:00:00 2001 From: Olivier Verdier Date: Mon, 2 Oct 2023 22:48:01 +0200 Subject: [PATCH 1/6] _adddiag! should be _adddiag --- src/addition.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/addition.jl b/src/addition.jl index 94f8797..08c453a 100644 --- a/src/addition.jl +++ b/src/addition.jl @@ -2,8 +2,8 @@ # between pdmat and pdmat +(a::PDMat, b::AbstractPDMat) = PDMat(a.mat + Matrix(b)) -+(a::PDiagMat, b::AbstractPDMat) = PDMat(_adddiag!(Matrix(b), a.diag)) -+(a::ScalMat, b::AbstractPDMat) = PDMat(_adddiag!(Matrix(b), a.value)) ++(a::PDiagMat, b::AbstractPDMat) = PDMat(_adddiag(Matrix(b), a.diag)) ++(a::ScalMat, b::AbstractPDMat) = PDMat(_adddiag(Matrix(b), a.value)) if HAVE_CHOLMOD +(a::PDSparseMat, b::AbstractPDMat) = PDMat(a.mat + Matrix(b)) end From 8e59a200d478311d014728f5c27250dce5e69a3e Mon Sep 17 00:00:00 2001 From: Olivier Verdier Date: Tue, 3 Oct 2023 09:23:03 +0200 Subject: [PATCH 2/6] better fix for addition Diag/ScalMat + AbstractPDMat --- src/addition.jl | 4 ++-- test/addition.jl | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/addition.jl b/src/addition.jl index 08c453a..ab4cbc0 100644 --- a/src/addition.jl +++ b/src/addition.jl @@ -2,8 +2,8 @@ # between pdmat and pdmat +(a::PDMat, b::AbstractPDMat) = PDMat(a.mat + Matrix(b)) -+(a::PDiagMat, b::AbstractPDMat) = PDMat(_adddiag(Matrix(b), a.diag)) -+(a::ScalMat, b::AbstractPDMat) = PDMat(_adddiag(Matrix(b), a.value)) ++(a::PDiagMat, b::AbstractPDMat) = PDMat(_adddiag!(Matrix(b), a.diag, true)) ++(a::ScalMat, b::AbstractPDMat) = PDMat(_adddiag!(Matrix(b), a.value)) if HAVE_CHOLMOD +(a::PDSparseMat, b::AbstractPDMat) = PDMat(a.mat + Matrix(b)) end diff --git a/test/addition.jl b/test/addition.jl index 9de57f2..728c157 100644 --- a/test/addition.jl +++ b/test/addition.jl @@ -2,6 +2,13 @@ using PDMats + +struct ScalMat2D{T<:Real} <: AbstractPDMat{T} + value::T +end + +Base.Matrix(a::ScalMat2D) = Matrix(Diagonal(fill(a.value, 2))) + @testset "addition" begin for T in (Float64, Float32) printstyled("Testing addition with eltype = $T\n"; color=:blue) @@ -33,4 +40,8 @@ using PDMats @test Matrix(pr) ≈ Matrix(p1) + pm4 end end + @testset "Abstract + Diag" for a in [PDiagMat([1,2]), ScalMat(2,1)] + M = ScalMat2D(1) + a + M + end end From 7f6686a3b98c47ba39f4e23f5c7cb83ad66a421b Mon Sep 17 00:00:00 2001 From: Olivier Verdier Date: Tue, 3 Oct 2023 09:34:44 +0200 Subject: [PATCH 3/6] test for `AbstractVector` change in `_adddiag!` --- src/utils.jl | 2 +- test/addition.jl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils.jl b/src/utils.jl index 3e1453f..3663cc4 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -30,7 +30,7 @@ function _adddiag!(a::Union{Matrix, SparseMatrixCSC}, v::Real) return a end -function _adddiag!(a::Union{Matrix, SparseMatrixCSC}, v::Vector, c::Real) +function _adddiag!(a::Union{Matrix, SparseMatrixCSC}, v::AbstractVector, c::Real) @check_argdims eachindex(v) == axes(a, 1) == axes(a, 2) if c == one(c) for i in eachindex(v) diff --git a/test/addition.jl b/test/addition.jl index 728c157..75f55a7 100644 --- a/test/addition.jl +++ b/test/addition.jl @@ -40,7 +40,7 @@ Base.Matrix(a::ScalMat2D) = Matrix(Diagonal(fill(a.value, 2))) @test Matrix(pr) ≈ Matrix(p1) + pm4 end end - @testset "Abstract + Diag" for a in [PDiagMat([1,2]), ScalMat(2,1)] + @testset "Abstract + Diag" for a in [PDiagMat([1,2]), ScalMat(2,1), PDiagMat(sparsevec([1.,0]))] M = ScalMat2D(1) a + M end From 4029adfb9ead3ef607dbb204b21db3dc079cecea Mon Sep 17 00:00:00 2001 From: Olivier Verdier Date: Tue, 3 Oct 2023 09:36:13 +0200 Subject: [PATCH 4/6] test for `AbstractVector` change in `_adddiag` --- src/utils.jl | 2 +- test/addition.jl | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/utils.jl b/src/utils.jl index 3663cc4..1ea5fef 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -46,7 +46,7 @@ end _adddiag(a::Union{Matrix, SparseMatrixCSC}, v::Real) = _adddiag!(copy(a), v) _adddiag(a::Union{Matrix, SparseMatrixCSC}, v::Vector, c::Real) = _adddiag!(copy(a), v, c) -_adddiag(a::Union{Matrix, SparseMatrixCSC}, v::Vector{T}) where {T<:Real} = _adddiag!(copy(a), v, one(T)) +_adddiag(a::Union{Matrix, SparseMatrixCSC}, v::AbstractVector{T}) where {T<:Real} = _adddiag!(copy(a), v, one(T)) function wsumsq(w::AbstractVector, a::AbstractVector) diff --git a/test/addition.jl b/test/addition.jl index 75f55a7..89a4980 100644 --- a/test/addition.jl +++ b/test/addition.jl @@ -44,4 +44,7 @@ Base.Matrix(a::ScalMat2D) = Matrix(Diagonal(fill(a.value, 2))) M = ScalMat2D(1) a + M end + A = randn(2,2) + M = PDMat(A*A') + M + PDiagMat(sparsevec([1.,0])) end From 5c053f0d07f477adeef3afaac8a9eddb8b3da732 Mon Sep 17 00:00:00 2001 From: Olivier Verdier Date: Tue, 3 Oct 2023 10:58:08 +0200 Subject: [PATCH 5/6] making actual @tests --- test/addition.jl | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/addition.jl b/test/addition.jl index 89a4980..c575cf4 100644 --- a/test/addition.jl +++ b/test/addition.jl @@ -42,9 +42,12 @@ Base.Matrix(a::ScalMat2D) = Matrix(Diagonal(fill(a.value, 2))) end @testset "Abstract + Diag" for a in [PDiagMat([1,2]), ScalMat(2,1), PDiagMat(sparsevec([1.,0]))] M = ScalMat2D(1) - a + M + @test Matrix(a + M) == Matrix(a) + Matrix(M) + end + @testset "PDMat + PDiagMat(sparse)" begin + A = randn(2, 2) + M = PDMat(A * A') + Dsp = PDiagMat(sparsevec([1.0, 0])) + @test Matrix(M + Dsp) == Matrix(M) + Matrix(Dsp) end - A = randn(2,2) - M = PDMat(A*A') - M + PDiagMat(sparsevec([1.,0])) end From d10d1a767d0ffcc0ad931d028a5ec2321ec43a44 Mon Sep 17 00:00:00 2001 From: David Widmann Date: Tue, 3 Oct 2023 17:54:18 +0200 Subject: [PATCH 6/6] Additional fix and tests --- src/utils.jl | 2 +- test/addition.jl | 48 ++++++++++++++++++++++++++---------------------- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/utils.jl b/src/utils.jl index 1ea5fef..c85d938 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -45,7 +45,7 @@ function _adddiag!(a::Union{Matrix, SparseMatrixCSC}, v::AbstractVector, c::Real end _adddiag(a::Union{Matrix, SparseMatrixCSC}, v::Real) = _adddiag!(copy(a), v) -_adddiag(a::Union{Matrix, SparseMatrixCSC}, v::Vector, c::Real) = _adddiag!(copy(a), v, c) +_adddiag(a::Union{Matrix, SparseMatrixCSC}, v::AbstractVector, c::Real) = _adddiag!(copy(a), v, c) _adddiag(a::Union{Matrix, SparseMatrixCSC}, v::AbstractVector{T}) where {T<:Real} = _adddiag!(copy(a), v, one(T)) diff --git a/test/addition.jl b/test/addition.jl index c575cf4..32b48e1 100644 --- a/test/addition.jl +++ b/test/addition.jl @@ -3,11 +3,16 @@ using PDMats -struct ScalMat2D{T<:Real} <: AbstractPDMat{T} +# New AbstractPDMat type for the tests below +# Supports only functions needed in the tests below +struct ScalMat3D{T<:Real} <: AbstractPDMat{T} value::T end - -Base.Matrix(a::ScalMat2D) = Matrix(Diagonal(fill(a.value, 2))) +Base.Matrix(a::ScalMat3D) = Matrix(Diagonal(fill(a.value, 3))) +Base.size(::ScalMat3D) = (3, 3) +# Not generally correct +Base.:*(a::ScalMat3D, c::Real) = ScalMat3D(a.value * c) +Base.getindex(a::ScalMat3D, i::Int, j::Int) = i == j ? a.value : zero(a.value) @testset "addition" begin for T in (Float64, Float32) @@ -18,36 +23,35 @@ Base.Matrix(a::ScalMat2D) = Matrix(Diagonal(fill(a.value, 2))) pm1 = PDMat(M) pm2 = PDiagMat(V) - pm3 = ScalMat(3, X) - pm4 = X * I + pm3 = PDiagMat(sparse(V)) + pm4 = ScalMat(3, X) pm5 = PDSparseMat(sparse(M)) + pm6 = ScalMat3D(X) - pmats = Any[pm1, pm2, pm3] #, pm5] + pmats = Any[pm1, pm2, pm3, pm4, pm5, pm6] for p1 in pmats, p2 in pmats pr = p1 + p2 @test size(pr) == size(p1) @test Matrix(pr) ≈ Matrix(p1) + Matrix(p2) - pr = pdadd(p1, p2, convert(T, 1.5)) - @test size(pr) == size(p1) - @test Matrix(pr) ≈ Matrix(p1) + Matrix(p2) * convert(T, 1.5) + if p1 isa ScalMat3D + @test_broken pdadd(p1, p2, convert(T, 1.5)) + else + pr = pdadd(p1, p2, convert(T, 1.5)) + @test size(pr) == size(p1) + @test Matrix(pr) ≈ Matrix(p1) + Matrix(p2) * convert(T, 1.5) + end end for p1 in pmats - pr = p1 + pm4 - @test size(pr) == size(p1) - @test Matrix(pr) ≈ Matrix(p1) + pm4 + if p1 isa ScalMat3D + @test_broken p1 + X * I + else + pr = p1 + X * I + @test size(pr) == size(p1) + @test Matrix(pr) ≈ Matrix(p1) + X * I + end end end - @testset "Abstract + Diag" for a in [PDiagMat([1,2]), ScalMat(2,1), PDiagMat(sparsevec([1.,0]))] - M = ScalMat2D(1) - @test Matrix(a + M) == Matrix(a) + Matrix(M) - end - @testset "PDMat + PDiagMat(sparse)" begin - A = randn(2, 2) - M = PDMat(A * A') - Dsp = PDiagMat(sparsevec([1.0, 0])) - @test Matrix(M + Dsp) == Matrix(M) + Matrix(Dsp) - end end