Reorder input arguments of matrix_group; MatrixGroup -> (oscar-sy…

…stem#2587)

docs/src/Groups/matgroup.md

@@ -8,7 +8,7 @@ end
 # Matrix groups
 
 ```@docs
-matrix_group(m::Int, R::Ring, V::AbstractVector{T}; check::Bool=true) where T<:Union{MatElem,MatrixGroupElem}
+matrix_group(R::Ring, m::Int, V::AbstractVector{T}; check::Bool=true) where T<:Union{MatElem,MatrixGroupElem}
 MatrixGroup{RE<:RingElem, T<:MatElem{RE}}
 MatrixGroupElem{RE<:RingElem, T<:MatElem{RE}}
 base_ring(G::MatrixGroup)

experimental/GITFans/src/GITFans.jl

@@ -212,7 +212,7 @@ function action_on_target(Q::Matrix{Int}, G::PermGroup)
     end
 
     # Create the matrix group.
-    matgroup = matrix_group(n, QQ, matgens)
+    matgroup = matrix_group(QQ, n, matgens)
 
     # Create the group homomorphism.
     return hom(G, matgroup, permgens, gens(matgroup))

experimental/LinearQuotients/src/misc.jl

@@ -18,7 +18,7 @@ function subgroup_of_reflections(G::MatrixGroup)
       append!(g, collect(c))
     end
   end
-  return matrix_group(degree(G), base_ring(G), g)
+  return matrix_group(base_ring(G), degree(G), g)
 end
 
 # Check if G contains reflections

experimental/QuadFormAndIsom/src/embeddings.jl

@@ -374,7 +374,7 @@ function _subgroups_orbit_representatives_and_stabilizers_elementary(Vinq::TorQu
   # wanted)
   act_GV_Vp = FpMatrix[change_base_ring(base_ring(Qp), matrix(gg)) for gg in gens(GV)]
   act_GV_Qp = FpMatrix[solve(VptoQp.matrix, g*VptoQp.matrix) for g in act_GV_Vp]
-  MGp = matrix_group(dim(Qp), base_ring(Qp), act_GV_Qp)
+  MGp = matrix_group(base_ring(Qp), dim(Qp), act_GV_Qp)
   GVtoMGp = hom(GV, MGp, MGp.(act_GV_Qp); check = false)
   GtoMGp = compose(GtoGV, GVtoMGp)
 

src/Groups/libraries/atlasgroups.jl

@@ -75,7 +75,7 @@ function atlas_group(info::Dict)
     deg = GAP.Globals.DimensionOfMatrixGroup(G)
     iso = info[:base_ring_iso]
     ring = domain(iso)
-    matgrp = MatrixGroup(deg, ring)
+    matgrp = matrix_group(ring, deg)
     matgrp.ring_iso = iso
     matgrp.X = G
     return matgrp

src/Groups/matrices/MatGrp.jl

@@ -1,41 +1,25 @@
+matrix_group(F::Ring, m::Int) = MatrixGroup{elem_type(F), dense_matrix_type(elem_type(F))}(F, m)
 
-
-
-abstract type AbstractMatrixGroupElem <: GAPGroupElem{GAPGroup} end
-
-# NOTE: always defined are deg, ring and at least one between { X, gens, descr }
+# build a MatrixGroup given a list of generators, given as array of either MatrixGroupElem or AbstractAlgebra matrices
 """
-    MatrixGroup{RE<:RingElem, T<:MatElem{RE}} <: GAPGroup
-
-Type of groups `G` of `n x n` matrices over the ring `R`, where `n = degree(G)` and `R = base_ring(G)`.
+    matrix_group(R::Ring, m::Int, V::T...) where T<:Union{MatElem,MatrixGroupElem}
+    matrix_group(R::Ring, m::Int, V::AbstractVector{T}) where T<:Union{MatElem,MatrixGroupElem}
+    matrix_group(V::T...) where T<:Union{MatElem,MatrixGroupElem}
+    matrix_group(V::AbstractVector{T}) where T<:Union{MatElem,MatrixGroupElem}
 
-At the moment, only rings of type `fqPolyRepField` are supported.
+Return the matrix group generated by matrices in `V`. If the degree `m` and
+coefficient ring `R` are not given, then `V` must be non-empty
 """
-@attributes mutable struct MatrixGroup{RE<:RingElem, T<:MatElem{RE}} <: GAPGroup
-   deg::Int
-   ring::Ring
-   X::GapObj
-   gens::Vector{<:AbstractMatrixGroupElem}
-   descr::Symbol                       # e.g. GL, SL, symbols for isometry groups
-   ring_iso::MapFromFunc # Isomorphism from the Oscar base ring to the GAP base ring
-
-   MatrixGroup{RE,T}(m::Int, F::Ring) where {RE,T} = new{RE,T}(m,F)
-
-end
-
-MatrixGroup(m::Int, F::Ring) = MatrixGroup{elem_type(F), dense_matrix_type(elem_type(F))}(m,F)
-
-# build a MatrixGroup given a list of generators, given as array of either MatrixGroupElem or AbstractAlgebra matrices
-function MatrixGroup{RE,S}(m::Int, F::Ring, V::AbstractVector{T}; check::Bool=true) where {RE,S} where T<:Union{MatElem,AbstractMatrixGroupElem}
-   @req all(v -> size(v) == (m,m), V) "Matrix group generators must all square and of equal degree"
+function matrix_group(F::Ring, m::Int, V::AbstractVector{T}; check::Bool=true) where T<:Union{MatElem,AbstractMatrixGroupElem}
+   @req all(v -> size(v) == (m,m), V) "Matrix group generators must all be square and of equal degree"
    @req all(v -> base_ring(v) == F, V) "Matrix group generators must have the same base ring"
 
    # if T <: MatrixGroupElem, we can already assume that det(V[i]) != 0
    if T<:MatElem && check
      @req all(v -> is_unit(det(v)), V) "Matrix group generators must be invertible over their base ring"
    end
 
-   G = MatrixGroup(m,F)
+   G = matrix_group(F, m)
    L = Vector{elem_type(G)}(undef, length(V))
    for i in 1:length(V)
       if T<:MatElem
@@ -62,7 +46,15 @@ function MatrixGroup{RE,S}(m::Int, F::Ring, V::AbstractVector{T}; check::Bool=tr
    return G
 end
 
-MatrixGroup(m::Int, F::Ring, V::AbstractVector{T}) where T<:Union{MatElem,AbstractMatrixGroupElem} = MatrixGroup{elem_type(F), dense_matrix_type(elem_type(F))}(m,F,V)
+matrix_group(R::Ring, m::Int, V::T...; check::Bool=true) where T<:Union{MatElem,MatrixGroupElem} = matrix_group(R, m, collect(V); check)
+
+matrix_group(V::AbstractVector{T}; check::Bool=true) where T<:Union{MatElem,MatrixGroupElem} = matrix_group(base_ring(V[1]), nrows(V[1]), V; check)
+
+matrix_group(V::T...; check::Bool=true) where T<:Union{MatElem,MatrixGroupElem} = matrix_group(collect(V); check)
+
+# For `general_linear_group` etc. the degree comes first, so we should also provide that option
+matrix_group(m::Int, R::Ring) = matrix_group(R, m)
+matrix_group(m::Int, R::Ring, V; check::Bool = true) = matrix_group(R, m, V, check = check)
 
 
 # `MatrixGroup`: compare types, dimensions, and coefficient rings
@@ -71,56 +63,13 @@ function check_parent(G::T, g::GAPGroupElem) where T <: MatrixGroup
   return T === typeof(P) && G.deg == P.deg && G.ring == P.ring
 end
 
-# `MatrixGroup`: set dimension and ring of `G`
-function _oscar_group(obj::GapObj, G::MatrixGroup)
-  d = GAP.Globals.DimensionOfMatrixGroup(obj)
-  d == G.deg || error("requested dimension of matrices ($(G.deg)) does not match the given matrix dimension ($d)")
-
-  R = G.ring
-  iso = G.ring_iso
-  GAPWrap.IsSubset(codomain(iso), GAP.Globals.FieldOfMatrixGroup(obj)) || error("matrix entries are not in the requested ring ($(codomain(iso)))")
-
-  M = MatrixGroup(d, R)
-  M.X = obj
-  M.ring = R
-  M.ring_iso = iso
-  return M
-end
-
-
-function _as_subgroup_bare(G::T, H::GapObj) where {T <: MatrixGroup}
-  H1 = T(G.deg,G.ring)
+function _as_subgroup_bare(G::MatrixGroup, H::GapObj)
+  H1 = typeof(G)(base_ring(G), degree(G))
   H1.ring_iso = G.ring_iso
   H1.X = H
   return H1
 end
 
-# NOTE: at least one of the fields :elm and :X must always defined, but not necessarily both of them.
-"""
-    MatrixGroupElem{RE<:RingElem, T<:MatElem{RE}} <: AbstractMatrixGroupElem
-
-Elements of a group of type `MatrixGroup{RE<:RingElem, T<:MatElem{RE}}`
-"""
-mutable struct MatrixGroupElem{RE<:RingElem, T<:MatElem{RE}} <: AbstractMatrixGroupElem
-   parent::MatrixGroup{RE, T}
-   elm::T                         # Oscar matrix
-   X::GapObj                     # GAP matrix. If x isa MatrixGroupElem, then x.X = map_entries(x.parent.ring_iso, x.elm)
-
-   # full constructor
-   MatrixGroupElem{RE,T}(G::MatrixGroup{RE,T}, x::T, x_gap::GapObj) where {RE, T} = new{RE,T}(G, x, x_gap)
-
-   # constructor which leaves `X` undefined
-   MatrixGroupElem{RE,T}(G::MatrixGroup{RE,T}, x::T) where {RE, T} = new{RE,T}(G, x)
-
-   # constructor which leaves `elm` undefined
-   function MatrixGroupElem{RE,T}(G::MatrixGroup{RE,T}, x_gap::GapObj) where {RE, T}
-      z = new{RE,T}(G)
-      z.X = x_gap
-      return z
-   end
-
-end
-
 MatrixGroupElem(G::MatrixGroup{RE,T}, x::T, x_gap::GapObj) where {RE,T} = MatrixGroupElem{RE,T}(G, x, x_gap)
 
 MatrixGroupElem(G::MatrixGroup{RE,T}, x::T) where {RE, T} = MatrixGroupElem{RE,T}(G,x)
@@ -610,7 +559,7 @@ julia> gens(H)
 ```
 """
 function general_linear_group(n::Int, R::Ring)
-   G = MatrixGroup(n,R)
+   G = matrix_group(R, n)
    G.descr = :GL
    return G
 end
@@ -644,7 +593,7 @@ julia> gens(H)
 ```
 """
 function special_linear_group(n::Int, R::Ring)
-   G = MatrixGroup(n,R)
+   G = matrix_group(R, n)
    G.descr = :SL
    return G
 end
@@ -680,7 +629,7 @@ julia> gens(H)
 """
 function symplectic_group(n::Int, R::Ring)
    @req iseven(n) "The dimension must be even"
-   G = MatrixGroup(n,R)
+   G = matrix_group(R, n)
    G.descr = :Sp
    return G
 end
@@ -718,15 +667,15 @@ julia> gens(H)
 function orthogonal_group(e::Int, n::Int, R::Ring)
    if e==1
       @req iseven(n) "The dimension must be even"
-      G = MatrixGroup(n,R)
+      G = matrix_group(R, n)
       G.descr = Symbol("GO+")
    elseif e==-1
       @req iseven(n) "The dimension must be even"
-      G = MatrixGroup(n,R)
+      G = matrix_group(R, n)
       G.descr = Symbol("GO-")
    elseif e==0
       @req isodd(n) "The dimension must be odd"
-      G = MatrixGroup(n,R)
+      G = matrix_group(R, n)
       G.descr = :GO
    else
       throw(ArgumentError("Invalid description of orthogonal group"))
@@ -772,15 +721,15 @@ function special_orthogonal_group(e::Int, n::Int, R::Ring)
    characteristic(R) == 2 && return GO(e,n,R)
    if e==1
       @req iseven(n) "The dimension must be even"
-      G = MatrixGroup(n,R)
+      G = matrix_group(R, n)
       G.descr = Symbol("SO+")
    elseif e==-1
       @req iseven(n) "The dimension must be even"
-      G = MatrixGroup(n,R)
+      G = matrix_group(R, n)
       G.descr = Symbol("SO-")
    elseif e==0
       @req isodd(n) "The dimension must be odd"
-      G = MatrixGroup(n,R)
+      G = matrix_group(R, n)
       G.descr = :SO
    else
       throw(ArgumentError("Invalid description of orthogonal group"))
@@ -823,15 +772,15 @@ function omega_group(e::Int, n::Int, R::Ring)
    n==1 && return SO(e,n,R)
    if e==1
       @req iseven(n) "The dimension must be even"
-      G = MatrixGroup(n,R)
+      G = matrix_group(R, n)
       G.descr = Symbol("Omega+")
    elseif e==-1
       @req iseven(n) "The dimension must be even"
-      G = MatrixGroup(n,R)
+      G = matrix_group(R, n)
       G.descr = Symbol("Omega-")
    elseif e==0
       @req isodd(n) "The dimension must be odd"
-      G = MatrixGroup(n,R)
+      G = matrix_group(R, n)
       G.descr = :Omega
    else
       throw(ArgumentError("Invalid description of orthogonal group"))
@@ -866,7 +815,7 @@ julia> gens(H)
 function unitary_group(n::Int, q::Int)
    fl, a, b = is_prime_power_with_data(q)
    @req fl "The field size must be a prime power"
-   G = MatrixGroup(n,GF(b, 2*a))
+   G = matrix_group(GF(b, 2*a), n)
    G.descr = :GU
    return G
 end
@@ -892,7 +841,7 @@ julia> gens(H)
 function special_unitary_group(n::Int, q::Int)
    fl, a, b = is_prime_power_with_data(q)
    @req fl "The field size must be a prime power"
-   G = MatrixGroup(n,GF(b, 2*a))
+   G = matrix_group(GF(b, 2*a), n)
    G.descr = :SU
    return G
 end
@@ -905,26 +854,6 @@ const SO = special_orthogonal_group
 const GU = unitary_group
 const SU = special_unitary_group
 
-
-"""
-    matrix_group(m::Int, R::Ring, V::T...) where T<:Union{MatElem,MatrixGroupElem}
-    matrix_group(m::Int, R::Ring, V::AbstractVector{T}) where T<:Union{MatElem,MatrixGroupElem}
-    matrix_group(V::T...) where T<:Union{MatElem,MatrixGroupElem}
-    matrix_group(V::AbstractVector{T}) where T<:Union{MatElem,MatrixGroupElem}
-
-Return the matrix group generated by matrices in `V`. If the degree `m` and
-coefficient ring `R` are not given, then `V` must be non-empty
-"""
-function matrix_group(m::Int, R::Ring, V::AbstractVector{T}; check::Bool=true) where T<:Union{MatElem,MatrixGroupElem}
-   return MatrixGroup(m, R, V)
-end
-
-matrix_group(m::Int, R::Ring, V::T...; check::Bool=true) where T<:Union{MatElem,MatrixGroupElem} = matrix_group(m, R, collect(V); check)
-
-matrix_group(V::AbstractVector{T}; check::Bool=true) where T<:Union{MatElem,MatrixGroupElem} = matrix_group(nrows(V[1]), base_ring(V[1]), V; check)
-
-matrix_group(V::T...; check::Bool=true) where T<:Union{MatElem,MatrixGroupElem} = matrix_group(collect(V); check)
-
 ########################################################################
 #
 # Subgroups
@@ -966,19 +895,19 @@ end
 
 function Base.:^(H::MatrixGroup, y::MatrixGroupElem)
    if isdefined(H,:gens) && !isdefined(H,:X)
-      K = MatrixGroup(H.deg, H.ring)
+      K = matrix_group(base_ring(H), degree(H))
       K.gens = [y^-1*x*y for x in H.gens]
       for k in gens(K) k.parent = K end
    else
-      K = MatrixGroup(H.deg,H.ring)
+      K = matrix_group(base_ring(H), degree(H))
       K.X = H.X^y.X
    end
 
    return K
 end
 
 function Base.rand(rng::Random.AbstractRNG, C::GroupConjClass{S,T}) where S<:MatrixGroup where T<:MatrixGroup
-   H = MatrixGroup(C.X.deg,C.X.ring)
+   H = matrix_group(C.X.ring, C.X.deg)
    H.X = GAP.Globals.Random(GAP.wrap_rng(rng), C.CC)::GapObj
    return H
 end

src/Groups/matrices/iso_nf_fq.jl

@@ -21,7 +21,7 @@ function _isomorphic_group_over_finite_field(matrices::Vector{<:MatrixElem{T}};
 
    Fq, matrices_Fq, OtoFq = good_reduction(matrices, 2)
 
-   G = matrix_group(n, Fq, matrices_Fq)
+   G = matrix_group(Fq, n, matrices_Fq)
    N = order(G)
    if !is_divisible_by(Hecke._minkowski_multiple(K, n), N)
       error("Group is not finite")

src/Groups/matrices/linear_centralizer.jl

@@ -448,7 +448,7 @@ If `G` = `GL(n,F)` or `SL(n,F)`, then `f` = `nothing`. In this case, to get the
 function centralizer(G::MatrixGroup{T}, x::MatrixGroupElem{T}) where T <: FinFieldElem
    if isdefined(G,:descr) && (G.descr==:GL || G.descr==:SL)
       V,card = G.descr==:GL ? _centralizer_GL(x.elm) : _centralizer_SL(x.elm)
-      H = MatrixGroup(G.deg, G.ring, V)
+      H = matrix_group(base_ring(G), degree(G), V)
       set_attribute!(H, :order => ZZRingElem(card))
       return H, nothing          # do not return the embedding of the centralizer into G to do not compute G.X
    end