Change `MatRing` & `MatRingElem` to wrap a "native" matrix, not a `Matrix{T}`
I think we should change the implementation of Generic.MatRingElem from
struct MatRingElem{T <: NCRingElement} <: AbstractAlgebra.MatRingElem{T}
base_ring::NCRing
entries::Matrix{T}
function MatRingElem{T}(R::NCRing, A::Matrix{T}) where T <: NCRingElement
@assert elem_type(R) === T
return new{T}(R, A)
end
end
to something more like this (untested, but I hope at least the principle idea gets across)
struct MatRingElem{T <: NCRingElement} <: AbstractAlgebra.MatRingElem{T}
data::MatElem{T}
function MatRingElem{T}(A::MatElem{T}) where T <: NCRingElement
@assert A isa dense_matrix_type(T)
return new{T}(A)
end
end
function data(M::MatRingElem{T}) where T
return M.data::dense_matrix_type(T)
end
This way those MatRingElem are less useless / more efficient. After a quick glance a bunch of code should actually become simpler. And some code that currently is implement for Generic.MatRingElem and Generic.Mat simultaneously needs to be untangled, but that's fine, e.g.
number_of_rows(a::Union{Mat, MatRingElem}) = size(a.entries, 1)
number_of_columns(a::Union{Mat,MatRingElem}) = size(a.entries, 2)
Base.@propagate_inbounds getindex(a::Union{Mat, MatRingElem}, r::Int, c::Int) = a.entries[r, c]
Base.@propagate_inbounds function setindex!(a::Union{Mat, MatRingElem}, d::NCRingElement,
r::Int, c::Int)
a.entries[r, c] = base_ring(a)(d)
end
Base.isassigned(a::Union{Mat,MatRingElem}, i, j) = isassigned(a.entries, i, j)
Note that we define
const MatrixElem{T} = Union{MatElem{T}, MatRingElem{T}}
and therefore when doing this untangling we need to inspect a bunch of methods taking an MatrixElem to see if they should perhaps be restricted to just MatElem.
In fact once we have untangled this, we have a very neat and efficient way to go from a MatRingElem to its "underlying" MatElem, and then can use that to avoid most if not all uses of MatrixElem in AbstractAlgebra and elsewhere.
CC @JohnAAbbott
I agree, we should change this.
See PR #2207 for a start on this, but it needs to be finished (CC @JohnAAbbott )
From triage: @JohnAAbbott hopes to finish this today.