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

eltype of Symmetric or Hermitian matrix of matrices is AbstractMatrix #54468

Open
araujoms opened this issue May 14, 2024 · 9 comments
Open

eltype of Symmetric or Hermitian matrix of matrices is AbstractMatrix #54468

araujoms opened this issue May 14, 2024 · 9 comments
Labels

Comments

@araujoms
Copy link
Contributor

MWE:

using LinearAlgebra
m = Symmetric(fill(ones(2,2), 2, 2))
eltype(m)
n = Hermitian(fill(ones(2,2), 2, 2))
eltype(n)

On the other hand, if we do this with UpperTriangular or LowerTriangular the eltype is Matrix{Float64}, as expected.

It's not a burning issue, but prevents the faster kronecker product introduced in #53186 from working on Symmetric matrices of matrices, and requires the less elegant syntax used in #54413.

I've tried to understand the problem, but it seems to come directly from the constructor so I'm stuck.

@inkydragon inkydragon added the domain:linear algebra Linear algebra label May 15, 2024
@jishnub
Copy link
Contributor

jishnub commented May 15, 2024

Does the result improve if the eltype is a Union of three types? That's generally the best that this may be narrowed down to.

@araujoms
Copy link
Contributor Author

araujoms commented May 15, 2024

I don't know, it depends on whether promote_op gives a sensible answer when given this Union.

The issue is that I need to allocate a matrix to store the result of the kronecker product, which I do with Matrix{promote_op(*, T, S)}. Now if T,S == Matrix{Float64} this gives me a Matrix{Matrix{Float64}}, which is exactly what I need. But if T,S == AbstractMatrix this gives me a Matrix{Any} which ruins everything.

EDIT: I just tested, and it does work. Let T = Union{Symmetric{Float64, Matrix{Float64}}, Transpose{Float64, Matrix{Float64}}, Matrix{Float64}}. Then promote_op(*, T, T) == Matrix{Float64} and everything is fine. Ditto if we have Hermitian and Adjoint instead of Symmetric and Transpose.

@dkarrasch
Copy link
Member

The reason is that that Symmetric wrappers only look at one, say the upper, triangle, and as for elements of the lower triangle, we pick elements from the upper and transpose them. Similarly for Hermitian. For Number elements, that transpose/adjoint doesn't make a type difference, but for matrix elements, the obtained result is a Transpose/Adjoint, which combined with the original matrices returns the AbstractMatrix. The function to look at is

function symmetric_type(::Type{T}) where {S<:AbstractMatrix, T<:AbstractMatrix{S}}
    return Symmetric{AbstractMatrix, T}
end

so it seems nobody had a better answer so far.

@araujoms
Copy link
Contributor Author

araujoms commented May 15, 2024

Thanks for the information. AbstractMatrix still seems like a bad choice, as it erases information. Union{Matrix{T},Transpose{Matrix{T}},Symmetric{Matrix{T}}} would still make it possible to allocate the necessary matrix.

@araujoms
Copy link
Contributor Author

On a second thought, the easiest solution would be for promote_op(*,AbstractMatrix,AbstractMatrix) to return AbstractMatrix. Does anything depend on it returning Any?

@dkarrasch
Copy link
Member

On a second thought, the easiest solution would be for promote_op(*,AbstractMatrix,AbstractMatrix) to return AbstractMatrix.

Good question why that is not the case.

@oscardssmith
Copy link
Member

not really, promote_op of almost any abstract type will return Any because the compiler can't prove that no one will introduce a new method.

@araujoms
Copy link
Contributor Author

Do you mean that someone could define a new Prank subtype of AbstractMatrix, and define a method for * that takes two Pranks to a String?

I suppose that's possible, but I don't see why promote_type should indulge this behaviour. Isn't type inference supposed to be just a heuristic anyway?

@oscardssmith
Copy link
Member

promote_op uses type inference, and while type inference is a heuristic, it is required to be a conservative heuristic. Specifically, type inference is always allowed to return a non-concrete supertype of the correct result, but it would be a very serious bug if type inference ever returned an incorrect result.

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

No branches or pull requests

5 participants