Matrix groups: cleanup some use of frobenius (oscar-system#4079)

* Matrix groups: cleanup some use of frobenius

Also added a comment on how conjugate_transpose could be
made more efficient if needed

* Fix precompile error (courtesy of @thofma)

* fix `_type` -> `Val(_type)`

* fix `frobenius` application

* add return statement

* fix `conjugate_transpose` indices

---------

Co-authored-by: Johannes Schmitt <schmitt@mathematik.uni-kl.de>
Co-authored-by: Morgan Rodgers <morgan.joaquin@gmail.com>

src/Groups/matrices/matrix_manipulation.jl

@@ -2,7 +2,7 @@
 # TODO : in this file, some functions for matrices and vectors are defined just to make other files work,
 # such as forms.jl, transform_form.jl, linear_conjugate.jl and linear_centralizer.jl
 # TODO : functions in this file are only temporarily, and often inefficient.
-# TODO: once similar working methods are defined in other files or packages (e.g. Hecke), 
+# TODO: once similar working methods are defined in other files or packages (e.g. Hecke),
 # functions in this file are to be removed / moved / replaced
 # TODO: when this happens, files mentioned above need to be modified too.
 
@@ -37,12 +37,22 @@ end
     conjugate_transpose(x::MatElem{T}) where T <: FinFieldElem
 
 If the base ring of `x` is `GF(q^2)`, return the matrix `transpose( map ( y -> y^q, x) )`.
- An exception is thrown if the base ring does not have even degree.
+An exception is thrown if the base ring does not have even degree.
 """
 function conjugate_transpose(x::MatElem{T}) where T <: FinFieldElem
-   @req iseven(degree(base_ring(x))) "The base ring must have even degree"
-   e = div(degree(base_ring(x)),2)
-   return transpose(map(y -> frobenius(y,e),x))
+  e = degree(base_ring(x))
+  @req iseven(e) "The base ring must have even degree"
+  e = div(e, 2)
+
+  y = similar(x, ncols(x), nrows(x))
+  for i in 1:ncols(x), j in 1:nrows(x)
+    # This code could be *much* faster, by precomputing the Frobenius map
+    # once; see also FrobeniusCtx in Hecke (but that does not yet support all
+    # finite field types at the time this comment was written).
+    # If you need this function to be faster, talk to Claus or Max.
+    y[i,j] = frobenius(x[j,i],e)
+  end
+  return y
 end
 
 
@@ -124,14 +134,12 @@ Return `false` if `B` is not a square matrix, or the field has not even degree.
 """
 function is_hermitian(B::MatElem{T}) where T <: FinFieldElem
    n = nrows(B)
-   n==ncols(B) || return false
+   n == ncols(B) || return false
    e = degree(base_ring(B))
    iseven(e) ? e = div(e,2) : return false
 
-   for i in 1:n
-      for j in i:n
-         B[i,j]==frobenius(B[j,i],e) || return false
-      end
+   for i in 1:n, j in i:n
+      B[i,j] == frobenius(B[j,i],e) || return false
    end
 
    return true
@@ -151,7 +159,7 @@ function _is_scalar_multiple_mat(x::MatElem{T}, y::MatElem{T}) where T <: RingEl
          return y == h*x ? (true,h) : (false, nothing)
       end
    end
-  
+
    # at this point, x must be zero
    return y == 0 ? (true, F(1)) : (false, nothing)
 end

src/Groups/matrices/transform_form.jl

@@ -18,7 +18,7 @@
 # TODO: it would be better if this is deterministic. This depends on gen(F) and is_square(F).
 
 function _solve_eqn(a::T, b::T, c::T) where T <: FinFieldElem
-   F = parent(a)  
+   F = parent(a)
    for x in F
       s = (c - a*x^2)*b^-1
       fl, t = is_square_with_sqrt(s)
@@ -49,7 +49,11 @@ function _find_radical(B::MatElem{T}, F::Field, nr::Int, nc::Int; e::Int=0, _is_
 #   A = matrix(vcat(type_vector[embU(v) for v in gens(U)], type_vector[embK(v) for v in gens(K)] ))
 
    if _is_symmetric
-      return A*B*transpose(map(y -> frobenius(y,e),A)), A, d
+      if e == 0
+         return A*B*transpose(A), A, d
+      else
+         return A*B*conjugate_transpose(A), A, d
+      end
    else
       A = transpose(A)
       return B*A, A, d
@@ -61,10 +65,10 @@ end
 
 
 
-# returns D, A such that A*B*transpose(frobenius(A)) = D and 
+# returns D, A such that A*B*transpose(frobenius(A)) = D and
 # D is diagonal matrix (or with blocks [0 1 s 0])
 # f = dimension of the zero block in B in the isotropic case
-function _block_anisotropic_elim(B::MatElem{T}, _type::Symbol; isotr=false, f=0)  where T <: FinFieldElem
+function _block_anisotropic_elim(B::MatElem{T}, ::Val{_type}; isotr=false, f=0)  where {T <: FinFieldElem, _type}
 
    d = nrows(B)
    F = base_ring(B)
@@ -75,17 +79,17 @@ function _block_anisotropic_elim(B::MatElem{T}, _type::Symbol; isotr=false, f=0)
    if _type==:symmetric
       degF=0
       s=1
+      star = X -> transpose(X)
    elseif _type==:alternating
       degF=0
       s=-1
+      star = X -> transpose(X)
    elseif _type==:hermitian
       degF=div(degree(F),2)
       s=1
+      star = X -> conjugate_transpose(X)
    end
 
-   # conjugate transpose in hermitian case
-   # transpose in the other cases
-   star(X) = transpose(map(y -> frobenius(y,degF),X))
 
    if isotr
       q = characteristic(F)^degF
@@ -117,7 +121,7 @@ function _block_anisotropic_elim(B::MatElem{T}, _type::Symbol; isotr=false, f=0)
             push!(Aarray, matrix(F,2,2,[1,0,0,1]))
          end
       end
-      B0,A0 = _block_anisotropic_elim(Bprime,_type)
+      B0,A0 = _block_anisotropic_elim(Bprime, Val(_type))
       B1 = cat(Barray..., dims=(1,2))
       B1 = cat(B1,B0,dims=(1,2))
       C = C^-1
@@ -143,15 +147,15 @@ function _block_anisotropic_elim(B::MatElem{T}, _type::Symbol; isotr=false, f=0)
       U1 = U[1:f, 1:e]
       U2 = U[1:f, e+1:c]
       Z = V-s*U1*B1^-1*star(U1)
-      D1,A1 = _block_anisotropic_elim(B1,_type)
+      D1,A1 = _block_anisotropic_elim(B1, Val(_type))
       Temp = zero_matrix(F,d-e,d-e)
       Temp[1:c-e, c-e+1:c-e+f] = s*star(U2)
       Temp[c-e+1:c-e+f, 1:c-e] = U2
       Temp[c-e+1:c-e+f, c-e+1:c-e+f] = Z
       if c-e==0
-         D2,A2 = _block_anisotropic_elim(Temp,_type)
+         D2,A2 = _block_anisotropic_elim(Temp, Val(_type))
       else
-         D2,A2 = _block_anisotropic_elim(Temp, _type; isotr=true, f=c-e)
+         D2,A2 = _block_anisotropic_elim(Temp, Val(_type); isotr=true, f=c-e)
       end
       Temp = hcat(-U1*B1^-1, zero_matrix(F,f,c-e))*A0
       Temp = vcat(A0,Temp)
@@ -167,7 +171,7 @@ end
 # assume B is nondegenerate
 
 
-# returns D, A such that A*B*transpose(frobenius(A)) = D and 
+# returns D, A such that A*B*transpose(frobenius(A)) = D and
 # D is diagonal matrix (or with blocks [0 1 s 0])
 # f = dimension of the zero block in B in the isotropic case
 function _block_herm_elim(B::MatElem{T}, _type) where T <: FinFieldElem
@@ -181,9 +185,9 @@ function _block_herm_elim(B::MatElem{T}, _type) where T <: FinFieldElem
    c = Int(ceil(d/2))
    B2 = B[1:c, 1:c]
    if B2==0
-      D,A = _block_anisotropic_elim(B,_type; isotr=true, f=c)
+      D,A = _block_anisotropic_elim(B, Val(_type); isotr=true, f=c)
    else
-      D,A = _block_anisotropic_elim(B,_type)
+      D,A = _block_anisotropic_elim(B, Val(_type))
    end
 
    return D,A
@@ -388,7 +392,7 @@ function is_congruent(f::SesquilinearForm{T}, g::SesquilinearForm{T}) where T <:
    f.descr==g.descr || return false, nothing
    n = nrows(gram_matrix(f))
    F = base_ring(f)
-   
+
    if f.descr==:quadratic
       if iseven(characteristic(F))            # in this case we use the GAP algorithms
          Bg = preimage_matrix(_ring_iso(g), GAP.Globals.BaseChangeToCanonical(g.X))