gap-packages / ClassicalMaximals

Maximal subgroups of classical groups
Other
0 stars 8 forks source link

Revise tests to check forms directly #91

Closed fingolfin closed 2 years ago

fingolfin commented 2 years ago

Testing IsSubset(G, elm) can be slow in GAP if G is a matrix group of "large" degree. This is because GAP resorts to testing membership using a stabilizer chain in an isomorphic permutation group, even if G is a linear/unitary/orthogonal/symplectic group. So we replace that check with one that actually tests using the invariant forms of the groups in question.

This is work in progress, and waiting for PR #88 and needs a few functions such as SpIsotropicImprimitives to set invariant forms -- which PR #89 seems to do.

Checklist for the reviewer

General

Functions constructing generators of maximal subgroups

Functions assembling the list of all maximal subgroups of a certain group

The reviewer doesn't need to compare our results to magma's results. That's the job of the person implementing the code.

fingolfin commented 2 years ago

@TristanPfersdorff these tests caught one omission in SubfieldSp (it did not always set the invariant form, fixed in this PR), and potentially also a "real" bug in SymplecticSubfieldSU: for inputs n=2,q=4 the returned group does not preserve the given sesquilinear form. Indeed, according to a brute force search, it doesn't seem to preserve any such form:

gap> G:=SymplecticSubfieldSU(2,4);;
gap> Filtered(GF(4)^[2,2], M -> ForAll(GeneratorsOfGroup(G), g -> g*M*HermitianConjugate(g,2) = M));
[ [ [ 0*Z(2), 0*Z(2) ], [ 0*Z(2), 0*Z(2) ] ] ]
fingolfin commented 2 years ago

Oh no wait, I misunderstood the function arguments, the base field is GF(4) and the big field is GF(16). OK, so trying again:

gap> G:=SymplecticSubfieldSU(2,4);;
gap> forms:=Filtered(GF(4)^[2,2], M -> ForAll(GeneratorsOfGroup(G), g -> g*M*HermitianConjugate(g,4) = M));
[ [ [ 0*Z(2), 0*Z(2) ], [ 0*Z(2), 0*Z(2) ] ], [ [ 0*Z(2), Z(2)^0 ], [ Z(2)^0, 0*Z(2) ] ],
  [ [ 0*Z(2), Z(2^2) ], [ Z(2^2), 0*Z(2) ] ], [ [ 0*Z(2), Z(2^2)^2 ], [ Z(2^2)^2, 0*Z(2) ] ] ]
gap> Display(forms[2]);
 . 1
 1 .
gap> Display(InvariantSesquilinearForm(G).matrix);
 . 1
 1 .
gap> CheckSesquilinearForm(G);
false

The bug was in CheckSesquilinearForm, fixed now.

codecov[bot] commented 2 years ago

Codecov Report

Merging #91 (b40ffd9) into main (9365a36) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #91   +/-   ##
=======================================
  Coverage   96.15%   96.15%           
=======================================
  Files          14       14           
  Lines        2734     2734           
=======================================
  Hits         2629     2629           
  Misses        105      105           
Impacted Files Coverage Δ
gap/SubfieldMatrixGroups.gi 97.82% <100.00%> (ø)