Skip to content
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

Cannot convert an object of type Matrix{Float32} to an object of type UpperTriangular{Float32, Matrix{Float32}} when testing PDMats #1198

Closed
KristofferC opened this issue Feb 10, 2025 · 5 comments
Milestone

Comments

@KristofferC
Copy link
Member

PkgEval log: https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/7825364_vs_d63aded/PDMats.primary.log

* kron 
Kronecker product: Error During Test at /home/pkgeval/.julia/packages/PDMats/iX2rk/test/kron.jl:14
  Got exception outside of a @test
  MethodError: Cannot `convert` an object of type Matrix{Float32} to an object of type UpperTriangular{Float32, Matrix{Float32}}
  The function `convert` exists, but no method is defined for this combination of argument types.
  
  Closest candidates are:
    UpperTriangular{T, S}(::Any) where {T, S<:AbstractMatrix{T}}
     @ LinearAlgebra /opt/julia/share/julia/stdlib/v1.12/LinearAlgebra/src/triangular.jl:20
    convert(::Type{<:UpperTriangular}, !Matched::SparseArrays.AbstractSparseMatrixCSC)
     @ SparseArrays /opt/julia/share/julia/stdlib/v1.12/SparseArrays/src/sparsematrix.jl:990
    convert(::Type{T}, !Matched::Union{LinearAlgebra.AbstractTriangular, Bidiagonal, Diagonal}) where T<:UpperTriangular
     @ LinearAlgebra /opt/julia/share/julia/stdlib/v1.12/LinearAlgebra/src/special.jl:75
    ...
  
  Stacktrace:
    [1] PDMat{Float32, UpperTriangular{Float32, Matrix{Float32}}}(m::Matrix{Float32}, c::Cholesky{Float32, UpperTriangular{Float32, Matrix{Float32}}})
      @ PDMats ~/.julia/packages/PDMats/iX2rk/src/pdmat.jl:15
    [2] PDMat(mat::Matrix{Float32}, chol::Cholesky{Float32, UpperTriangular{Float32, Matrix{Float32}}})
      @ PDMats ~/.julia/packages/PDMats/iX2rk/src/pdmat.jl:22
    [3] kron(A::PDMat{Float32, Matrix{Float32}}, B::PDMat{Float32, Matrix{Float32}})
      @ PDMats ~/.julia/packages/PDMats/iX2rk/src/pdmat.jl:116
    [4] _pd_kron_compare(A::PDMat{Float32, Matrix{Float32}}, B::PDMat{Float32, Matrix{Float32}})
      @ Main ~/.julia/packages/PDMats/iX2rk/test/kron.jl:6
    [5] macro expansion
@KristofferC KristofferC added this to the 1.12 milestone Feb 10, 2025
@dkarrasch
Copy link
Member

This is caused by the fact that kron of two UpperTriangular matrices returns an UpperTriangular matrix now. So, in

Base.kron(A::PDMat, B::PDMat) = PDMat(kron(A.mat, B.mat), Cholesky(kron(A.chol.U, B.chol.U), 'U', A.chol.info))

we get from kron(A.chol.U, B.chol.U) an UpperTriangular, which then in

struct PDMat{T<:Real,S<:AbstractMatrix{T}} <: AbstractPDMat{T}
    mat::S
    chol::Cholesky{T,S}
...

forces S<:UpperTriangular, so it also tries to convert kron(A.mat, B.mat) to UpperTriangular. I see three options: (a) unwrap the triangular wrapper inside Cholesky, or (b) relax the type equality between mat and chol, or (c) introduce a method _full! which does nothing on v1.12-, and calls full! on v1.12+, and call it on the result of kron(A.chol.U, B.chol.U).

@jishnub @andreasnoack

@jishnub
Copy link
Member

jishnub commented Feb 10, 2025

(b) makes sense to me. I doubt there will always be a correspondence between the matrix types, and the other options seem like patches to fix this specific case.

@andreasnoack
Copy link
Member

I prefer something like (c). Unwrapping the triangular wrapper after kron seems right to me. Then you'll benefit from the the structure during kron but when wrapping the result in Cholesky, the wrapper should be gone as it it is implied by the Cholesky. I think the type equality makes sense and should be enforced.

@dkarrasch
Copy link
Member

Just to be clear: both options (b) and (c) would need to done within PDMats.jl. I'll open an issue and hope somebody picks it up.

@KristofferC KristofferC changed the title Cannot convert an object of type Matrix{Float32} to an object of type UpperTriangular{Float32, Matrix{Float32}} when testing PDMats Cannot convert an object of type Matrix{Float32} to an object of type UpperTriangular{Float32, Matrix{Float32}} when testing PDMats Feb 21, 2025
@devmotion
Copy link

This issue can be closed, the problem in PDMats is fixed on the master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants