Merge MatSpace and Generic.MatSpace (#1849)

docs/src/matrix.md

@@ -34,7 +34,7 @@ For the most part, one doesn't want to work directly with the `MatSpaceElem` typ
 but with an abstract type called `Generic.Mat` which includes `MatSpaceElem` and views
 thereof.
 
-Parents of generic matrices (matrix spaces) have type `Generic.MatSpace{T}`. Parents of
+Parents of generic matrices (matrix spaces) have type `MatSpace{T}`. Parents of
 matrices in a matrix algebra have type `Generic.MatRing{T}`.
 
 The dimensions and base ring $R$ of a generic matrix are stored in its parent object,
@@ -47,9 +47,10 @@ demand.
 ## Abstract types
 
 The generic matrix types (matrix spaces) belong to the abstract type
-`MatElem{T}` and the matrix space parent types belong to
-`MatSpace{T}`. Similarly the generic matrix algebra matrix types belong
-to the abstract type `MatRingElem{T}` and the parent types belong to
+`MatElem{T}` and the all matrix space parents are of the concrete type
+`MatSpace{T}`.
+On the other hand, the generic matrix algebra matrix types belong
+to the abstract type `MatRingElem{T}` and the parent types belong to the abstract
  `MatRing{T}` Note that both
 the concrete type of a matrix space parent object and the abstract class it belongs to
 have the name `MatElem`, therefore disambiguation is required to specify which is

docs/src/matrix_interface.md

@@ -31,9 +31,9 @@ performance.
 
 ## Types and parents
 
-AbstractAlgebra provides two abstract types for matrix spaces and their elements:
+AbstractAlgebra provides two types for matrix spaces and their elements:
 
-  * `MatSpace{T}` is the abstract type for matrix space parent types
+  * `MatSpace{T}` is the concrete type for matrix space parent types
   * `MatElem{T}` is the abstract type for matrix types belonging to a matrix space
 
 It also provides two abstract types for matrix algebras and their elements:

docs/src/visualizing_types.md

@@ -44,4 +44,4 @@ are defined over.
   - `Generic.LaurentSeriesFieldElem{T}` (`Generic.LaurentSeriesField{T}`)
   - `Generic.ResidueRingElem{T}` (`Generic.ResidueRing{T}`)
   - `Generic.FracFieldElem{T}` (`Generic.FracField{T}`)
-  - `Generic.Mat{T}` (`Generic.MatSpace{T}`)
+  - `Generic.Mat{T}` (`MatSpace{T}`)

src/AbstractAlgebra.jl

@@ -178,6 +178,8 @@ const NCRingElement = Union{NCRingElem, Integer, Rational, AbstractFloat}
 
 const FieldElement = Union{FieldElem, Rational, AbstractFloat}
 
+include("ConcreteTypes.jl")
+
 ###############################################################################
 #
 #   Fundamental interface for AbstractAlgebra

src/AbstractTypes.jl

@@ -93,8 +93,6 @@ abstract type ResidueField{T} <: Field end
 
 abstract type FracField{T} <: Field end
 
-abstract type MatSpace{T} <: Module{T} end
-
 abstract type MatRing{T} <: NCRing end
 
 abstract type FreeAssociativeAlgebra{T} <: NCRing end

src/ConcreteTypes.jl

@@ -0,0 +1,18 @@
+###############################################################################
+#
+#   Mat space
+#
+###############################################################################
+
+struct MatSpace{T <: NCRingElement} <: Module{T}
+  base_ring::NCRing
+  nrows::Int
+  ncols::Int
+
+  function MatSpace{T}(R::NCRing, r::Int, c::Int, cached::Bool = true) where T <: NCRingElement
+     # TODO/FIXME: `cached` is ignored and only exists for backwards compatibility
+     @assert elem_type(R) === T
+     (r < 0 || c < 0) && error("Dimensions must be non-negative")
+     return new{T}(R, r, c)
+  end
+end

src/Generic.jl

@@ -118,4 +118,7 @@ Base.@deprecate_binding ResidueRingElem EuclideanRingResidueRingElem
 @deprecate hom(M::DirectSumModule{T}, N::DirectSumModule{T}, mp::Vector{ModuleHomomorphism{T}}) where T hom_direct_sum(M, N, mp)
 @deprecate hom(A::DirectSumModule{T}, B::DirectSumModule{T}, M::Matrix{<:Map{<:AbstractAlgebra.FPModule{T}, <:AbstractAlgebra.FPModule{T}}}) where {T} hom_direct_sum(A, B, M)
 
+# Deprecated for 0.44
+#Base.@deprecate_binding MatSpace AbstractAlgebra.MatSpace false
+
 end # generic

src/Matrix.jl

@@ -10,7 +10,11 @@
 #
 ###############################################################################
 
-base_ring_type(::Type{<:MatSpace{T}}) where T <: NCRingElement = parent_type(T)
+elem_type(::Type{MatSpace{T}}) where {T <: NCRingElement} = dense_matrix_type(T)
+
+parent_type(::Type{<:MatElem{T}}) where {T <: NCRingElement} = MatSpace{T}
+
+base_ring_type(::Type{MatSpace{T}}) where T <: NCRingElement = parent_type(T)
 
 base_ring(a::MatSpace{T}) where {T <: NCRingElement} = a.base_ring::parent_type(T)
 
@@ -48,6 +52,41 @@ function _check_bases(a, b)
   return nothing
 end
 
+_checkbounds(i::Int, j::Int) = 1 <= j <= i
+
+_checkbounds(i::Int, j::AbstractVector{Int}) = all(jj -> 1 <= jj <= i, j)
+
+function _checkbounds(A, i::Union{Int, AbstractVector{Int}}, j::Union{Int, AbstractVector{Int}})
+  (_checkbounds(nrows(A), i) && _checkbounds(ncols(A), j)) ||
+            Base.throw_boundserror(A, (i, j))
+end
+
+function _checkbounds(A, rows::AbstractArray{Int}, cols::AbstractArray{Int})
+   Base.has_offset_axes(rows, cols) && throw(ArgumentError("offset arrays are not supported"))
+   isempty(rows) || _checkbounds(nrows(A), first(rows)) && _checkbounds(nrows(A), last(rows)) ||
+      throw(BoundsError(A, rows))
+   isempty(cols) || _checkbounds(ncols(A), first(cols)) && _checkbounds(ncols(A), last(cols)) ||
+      throw(BoundsError(A, cols))
+end
+
+function check_square(A::MatrixElem{T}) where T <: NCRingElement
+   is_square(A) || throw(DomainError(A, "matrix must be square"))
+   A
+end
+
+function check_square(S::MatSpace)
+   nrows(S) == ncols(S) || throw(DomainError(S, "matrices must be square"))
+   S
+end
+
+check_square(S::MatRing) = S
+
+###############################################################################
+#
+#   Parent object call overload
+#
+###############################################################################
+
 # create a zero matrix
 function (s::MatSpace{T})() where {T <: NCRingElement}
   return zero_matrix(base_ring(s), nrows(s), ncols(s))::eltype(s)
@@ -60,16 +99,12 @@ function (s::MatSpace{T})(a::MatrixElem{T}) where {T <: NCRingElement}
   M = s()  # zero matrix
   R = base_ring(s)
   if R == base_ring(a)
-    for i = 1:nrows(s)
-      for j = 1:ncols(s)
-        M[i, j] = a[i, j]
-      end
+    for i = 1:nrows(s), j = 1:ncols(s)
+      M[i, j] = a[i, j]
     end
   else
-    for i = 1:nrows(s)
-      for j = 1:ncols(s)
-        M[i, j] = R(a[i, j])
-      end
+    for i = 1:nrows(s), j = 1:ncols(s)
+      M[i, j] = R(a[i, j])
     end
   end
   return M
@@ -86,34 +121,30 @@ function (s::MatSpace)(b::NCRingElement)
   return M
 end
 
-_checkbounds(i::Int, j::Int) = 1 <= j <= i
-
-_checkbounds(i::Int, j::AbstractVector{Int}) = all(jj -> 1 <= jj <= i, j)
-
-function _checkbounds(A, i::Union{Int, AbstractVector{Int}}, j::Union{Int, AbstractVector{Int}})
-  (_checkbounds(nrows(A), i) && _checkbounds(ncols(A), j)) ||
-            Base.throw_boundserror(A, (i, j))
-end
+# convert a Julia matrix
+function (a::MatSpace{T})(b::AbstractMatrix{S}) where {T <: NCRingElement, S}
+  _check_dim(nrows(a), ncols(a), b)
+  R = base_ring(a)
 
-function _checkbounds(A, rows::AbstractArray{Int}, cols::AbstractArray{Int})
-   Base.has_offset_axes(rows, cols) && throw(ArgumentError("offset arrays are not supported"))
-   isempty(rows) || _checkbounds(nrows(A), first(rows)) && _checkbounds(nrows(A), last(rows)) ||
-      throw(BoundsError(A, rows))
-   isempty(cols) || _checkbounds(ncols(A), first(cols)) && _checkbounds(ncols(A), last(cols)) ||
-      throw(BoundsError(A, cols))
-end
+  # minor optimization for MatSpaceElem
+  if S === T && dense_matrix_type(T) === Generic.MatSpaceElem{T} && all(x -> R === parent(x), b)
+     return Generic.MatSpaceElem{T}(R, b)
+  end
 
-function check_square(A::MatrixElem{T}) where T <: NCRingElement
-   is_square(A) || throw(DomainError(A, "matrix must be square"))
-   A
+  # generic code
+  M = a()  # zero matrix
+  for i = 1:nrows(a), j = 1:ncols(a)
+     M[i, j] = R(b[i, j])
+  end
+  return M
 end
 
-function check_square(S::MatSpace)
-   nrows(S) == ncols(S) || throw(DomainError(S, "matrices must be square"))
-   S
+# convert a Julia vector
+function (a::MatSpace{T})(b::AbstractVector) where T <: NCRingElement
+  _check_dim(nrows(a), ncols(a), b)
+  return a(transpose(reshape(b, a.ncols, a.nrows)))
 end
 
-check_square(S::MatRing) = S
 
 ###############################################################################
 #
@@ -7023,7 +7054,11 @@ Return parent object corresponding to the space of $r\times c$ matrices over
 the ring $R$.
 """
 function matrix_space(R::NCRing, r::Int, c::Int; cached::Bool = true)
-   return Generic.matrix_space(R, r, c; cached)
+  # TODO: the 'cached' argument is ignored and mainly here for backwards compatibility
+  # (and perhaps future compatibility, in case we need it again)
+  (r < 0 || c < 0) && error("Dimensions must be non-negative")
+  T = elem_type(R)
+  return MatSpace{T}(R, r, c)
 end
 
 ###############################################################################

src/generic/GenericTypes.jl

@@ -1051,29 +1051,13 @@ end
 
 ###############################################################################
 #
-#   MatSpace / Mat
+#   Mat
 #
 ###############################################################################
 
-const NCRingElement = Union{NCRingElem, RingElement}
-
 # All MatSpaceElem's and views thereof
 abstract type Mat{T} <: MatElem{T} end
 
-# not really a mathematical ring
-struct MatSpace{T <: NCRingElement} <: AbstractAlgebra.MatSpace{T}
-   base_ring::NCRing
-   nrows::Int
-   ncols::Int
-
-   function MatSpace{T}(R::NCRing, r::Int, c::Int, cached::Bool = true) where T <: NCRingElement
-      # TODO/FIXME: `cached` is ignored and only exists for backwards compatibility
-      @assert elem_type(R) === T
-      (r < 0 || c < 0) && error("Dimensions must be non-negative")
-      return new{T}(R, r, c)
-   end
-end
-
 struct MatSpaceElem{T <: NCRingElement} <: Mat{T}
    base_ring::NCRing
    entries::Matrix{T}

src/generic/Matrix.jl

@@ -10,10 +10,6 @@
 #
 ###############################################################################
 
-parent_type(::Type{<:MatElem{T}}) where {T <: NCRingElement} = MatSpace{T}
-
-elem_type(::Type{MatSpace{T}}) where {T <: NCRingElement} = dense_matrix_type(T)
-
 @doc raw"""
     parent(a::AbstractAlgebra.MatElem)
 
@@ -43,20 +39,6 @@ dense_matrix_type(::Type{T}) where T <: NCRingElement = MatSpaceElem{T}
 #
 ###############################################################################
 
-@doc raw"""
-    number_of_rows(a::MatSpace)
-
-Return the number of rows of the given matrix space.
-"""
-number_of_rows(a::Generic.MatSpace) = a.nrows
-
-@doc raw"""
-    number_of_columns(a::MatSpace)
-
-Return the number of columns of the given matrix space.
-"""
-number_of_columns(a::Generic.MatSpace) = a.ncols
-
 number_of_rows(a::Union{Mat, MatRingElem}) = size(a.entries, 1)
 
 number_of_columns(a::Union{Mat,MatRingElem}) = size(a.entries, 2)
@@ -142,52 +124,6 @@ function promote_rule(::Type{S}, ::Type{U}) where {T <: NCRingElement, S <: Mat{
    promote_rule(T, U) == T ? MatSpaceElem{T} : Union{}
 end
 
-###############################################################################
-#
-#   Parent object call overload
-#
-###############################################################################
-
-
-
-# convert a Julia matrix
-function (a::Generic.MatSpace{T})(b::AbstractMatrix{S}) where {T <: NCRingElement, S}
-   _check_dim(nrows(a), ncols(a), b)
-   R = base_ring(a)
-
-   # minor optimization for MatSpaceElem
-   if S === T && dense_matrix_type(T) === MatSpaceElem{T} && all(x -> R === parent(x), b)
-      return MatSpaceElem{T}(R, b)
-   end
-
-   # generic code
-   M = a()  # zero matrix
-   for i = 1:nrows(a), j = 1:ncols(a)
-      M[i, j] = R(b[i, j])
-   end
-   return M
-end
-
-# convert a Julia vector
-function (a::Generic.MatSpace{T})(b::AbstractVector) where T <: NCRingElement
-   _check_dim(nrows(a), ncols(a), b)
-   return a(transpose(reshape(b, a.ncols, a.nrows)))
-end
-
-###############################################################################
-#
-#   matrix_space constructor
-#
-###############################################################################
-
-function matrix_space(R::AbstractAlgebra.NCRing, r::Int, c::Int; cached::Bool = true)
-   # TODO: the 'cached' argument is ignored and mainly here for backwards compatibility
-   # (and perhaps future compatibility, in case we need it again)
-   (r < 0 || c < 0) && error("Dimensions must be non-negative")
-   T = elem_type(R)
-   return Generic.MatSpace{T}(R, r, c)
-end
-
 function AbstractAlgebra.add!(A::Mat{T}, B::Mat{T}, C::Mat{T}) where T
   A.entries .= B.entries .+ C.entries
   return A

src/generic/imports.jl

@@ -88,6 +88,7 @@ import ..AbstractAlgebra: LowercaseOff
 import ..AbstractAlgebra: Map
 import ..AbstractAlgebra: NCRing
 import ..AbstractAlgebra: NCRingElem
+import ..AbstractAlgebra: NCRingElement
 import ..AbstractAlgebra: O
 import ..AbstractAlgebra: Perm
 import ..AbstractAlgebra: Ring

test/generic/Matrix-test.jl

@@ -65,7 +65,7 @@ AbstractAlgebra.divexact(x::F2Elem, y::F2Elem) = y.x ? x : throw(DivideError())
 Random.rand(rng::AbstractRNG, sp::Random.SamplerTrivial{F2}) = F2Elem(rand(rng, Bool))
 Random.gentype(::Type{F2}) = F2Elem
 
-const F2MatSpace = AbstractAlgebra.Generic.MatSpace{F2Elem}
+const F2MatSpace = MatSpace{F2Elem}
 
 (S::F2MatSpace)() = zero_matrix(F2(), S.nrows, S.ncols)
 
@@ -120,16 +120,16 @@ end
    @test S === matrix_space(R, 3, 3)
 
    @test elem_type(S) == Generic.MatSpaceElem{elem_type(R)}
-   @test elem_type(Generic.MatSpace{elem_type(R)}) == Generic.MatSpaceElem{elem_type(R)}
-   @test parent_type(Generic.MatSpaceElem{elem_type(R)}) == Generic.MatSpace{elem_type(R)}
-   @test base_ring_type(Generic.MatSpace{elem_type(R)}) == typeof(R)
+   @test elem_type(MatSpace{elem_type(R)}) == Generic.MatSpaceElem{elem_type(R)}
+   @test parent_type(Generic.MatSpaceElem{elem_type(R)}) == MatSpace{elem_type(R)}
+   @test base_ring_type(MatSpace{elem_type(R)}) == typeof(R)
    @test base_ring_type(Generic.MatSpaceElem{elem_type(R)}) == typeof(R)
    @test base_ring_type(S) == typeof(R)
    @test base_ring(S) == R
    @test nrows(S) == 3
    @test ncols(S) == 3
 
-   @test typeof(S) <: Generic.MatSpace
+   @test typeof(S) <: MatSpace
 
    f = S(t^2 + 1)
 
@@ -353,8 +353,8 @@ end
    @test base_ring(S) == R
 
    @test elem_type(S) == Generic.MatSpaceElem{elem_type(R)}
-   @test elem_type(Generic.MatSpace{elem_type(R)}) == Generic.MatSpaceElem{elem_type(R)}
-   @test parent_type(Generic.MatSpaceElem{elem_type(R)}) == Generic.MatSpace{elem_type(R)}
+   @test elem_type(MatSpace{elem_type(R)}) == Generic.MatSpaceElem{elem_type(R)}
+   @test parent_type(Generic.MatSpaceElem{elem_type(R)}) == MatSpace{elem_type(R)}
 
    @test dense_matrix_type(R) == elem_type(S)
    @test dense_matrix_type(R(1)) == elem_type(S)