Error in SLPforElementFuncsProjective.PSL2

[fingolfin]

Take this input group:

G := Group(Z(5)^0 * [
  [ [ 0, 2, 0, 2 ], [ 1, 4, 4, 4 ],
  [ 0, 4, 0, 0 ], [ 3, 2, 0, 0 ] ],
  [ [ 0, 2, 0, 2 ], [ 4, 4, 1, 4 ],
  [ 0, 1, 0, 0 ], [ 3, 3, 0, 0 ] ],
  [ [ 1, 0, 0, 0 ], [ 0, 4, 0, 0 ],
  [ 0, 0, 4, 0 ], [ 0, 0, 0, 1 ] ],
  [ [ 2, 0, 0, 0 ], [ 0, 2, 0, 0 ],
  [ 0, 0, 2, 0 ], [ 0, 0, 0, 2 ] ],
  [ [ 0, 0, 0, 1 ], [ 0, 2, 0, 0 ],
  [ 0, 0, 1, 0 ], [ 2, 0, 0, 0 ] ]
]);

Trying RecognizeGroup(G) often (= for many RNG seeds) prints one or more messages of the form

#I Strange, found less direct factors than expected!

which already indicates something is amiss.

But it sometimes also runs into an outright error:

gap> G:=Group(Z(5)^0*[[[0,2,0,2],[1,4,4,4],[0,4,0,0],[3,2,0,0]],[[0,2,0,2],[4,4,1,4],[0,1,0,0],[3,3,0,0]],[[1,0,0,0],[0,4,0,0],[0,0,4,0],[0,0,0,1]],[[2,0,0,0],[0,2,0,0],[0,0,2,0],[0,0,0,2]],[[0,0,0,1],[0,2,0,0],[0,0,1,0],[2,0,0,0]]]);
gap> seed:=115;;  Reset(GlobalMersenneTwister,seed);; Reset(GlobalRandomSource,seed);; RecognizeGroup(G);
Error, ^ cannot be used here to compute roots (use `RootInt' instead?) at GAPROOT/lib/arith.gi:623 called from
z ^ ((- log) * ri!.gcd.coeff1 / ri!.gcd.gcd)
  at GAPROOT/pkg/recog/gap/projective/classicalnatural.gi:3068 called from
slpforelement( ri )( ri, x ) at GAPROOT/pkg/recog/gap/base/recognition.gi:711 called from
SLPforElement( rifac, gg ) at GAPROOT/pkg/recog/gap/base/recognition.gi:733 called from
slpforelement( ri )( ri, x ) at GAPROOT/pkg/recog/gap/base/recognition.gi:711 called from
SLPforElement( rifac, gg ) at GAPROOT/pkg/recog/gap/base/recognition.gi:733 called from
slpforelement( ri )( ri, x ) at GAPROOT/pkg/recog/gap/base/recognition.gi:711 called from
SLPforElement( riker, n ) at GAPROOT/pkg/recog/gap/base/recognition.gi:753 called from
slpforelement( ri )( ri, x ) at GAPROOT/pkg/recog/gap/base/recognition.gi:711 called from
SLPforElement( ImageRecogNode( ri ), ImageElm( Homom( ri ), x!.el ) ) at GAPROOT/pkg/recog/gap/base/kernel.gi:46 called from
GenerateRandomKernelElementsAndOptionallyVerifyThem( ri, n, false ) at GAPROOT/pkg/recog/gap/base/kernel.gi:91 called from
CallFuncList( findgensNmeth( ri ).method, Concatenation( [ ri ], findgensNmeth( ri ).args ) ) at GAPROOT/pkg/recog/gap/base/recognition.gi:590 called from
RecogniseGeneric( G, FindHomDbMatrix, "", rec( ) ) at GAPROOT/pkg/recog/gap/base/recognition.gi:112 called from
RecogniseGroup( G ) at GAPROOT/ClassicalMaximals/tst/utils.g:8 called from
CheckSize( G ); at stream:5 called from
TestOrthogonalNormalizerInSL( 1, 4, 5 ); at *stdin*:5 called from
<function "unknown">( <arguments> )
 called from read-eval loop at *errin*:12
brk>

This is the code in question:

SLPforElementFuncsProjective.PSL2 := function(ri,x)
  local det,log,slp,y,z,pos,s;
  ri!.fakegens.count := ri!.fakegens.count + 1;
  if ri!.fakegens.count > 1000 then
      ri!.fakegens := RECOG.InitSLfake(ri!.field,2);
      ri!.fakegens.count := 0;
  fi;
  y := ri!.nicebas * x * ri!.nicebasi;
  det := DeterminantMat(y);
  if not IsOne(det) then
      z := PrimitiveRoot(ri!.field);
      log := LogFFE(det,z);
      y := y * z^(-log*ri!.gcd.coeff1/ri!.gcd.gcd);  # <-- error happens here
  fi;
brk>

So at this point det = z = Z(5), so log = 1, and ri!.gcd = rec( coeff1 := 1, coeff2 := 0, coeff3 := -2, coeff4 := 1, gcd := 2 ), so the exponent evaluates to -1/2.

It looks as if the code is trying to normalize the determinant of that matrix to one, by scaling the matrix with a square root of the inverse of that determinant -- but of course that doesn't exist in GF(5).

So the input matrix is not actually "in" PSL(2,5) (or rather, in SL(2,5)). Indeed, let's inspect some more on the break loop:

brk> Grp(ri);
<matrix group with 24 generators>
brk> Grp(ri).1;
[ [ 0*Z(5), Z(5)^2 ], [ Z(5)^0, Z(5)^2 ] ]
brk> Size(Grp(ri));
240
brk> Size(SL(2,5));
120

So the matrices generated GL(2,5), not SL(2,5). So this looks like a case of "mistaken identity": The code drew a wrong conclusion, it thinks it has (P)SL(2,5) but this is wrong.

[fingolfin]

I think we are getting here from the ClassicalNatural recognition method; it invokes RECOG.IsThisSL2Natural(ri,GeneratorsOfGroup(g),f) ... and apparently decides to go on.

Cranking up the info level, we see:

...
#I  SL2: Computing stabiliser chain.
#I  SL2: size is 240
#I  ClassicalNatural: this is PSL_2!
...

And that's... not right LOL. Now, several things:

• a comment at the start of IsThisSL2Natural says:

  # Checks quickly whether or not this is SL(2,f).
  # The answer is not guaranteed to be correct, this is Las Vegas.

• the relevant computation in our case then is done by this code snippet:

  q := Size(f);
  p := Characteristic(f);
  # For small q, comput the order of the group via a stabilizer chain.
  # Note that at this point we are usually working projective, and thus
  # scalars are factored out "implicitly". Thus the generators we are
  # looking at may generate a group which only contains SL2 as a subgroup.
  if q <= 11 then    # this could be increased if needed
      Info(InfoRecog,4,"SL2: Computing stabiliser chain.");
      S := StabilizerChain(Group(gens));
      Info(InfoRecog,4,"SL2: size is ",Size(S));
      return Size(S) mod (q*(q-1)*(q+1)) = 0;
  fi;

• clearly that doesn't test whether the group is SL2, but rather whether it contains SL2; it even explicitly states so and gives a reason... although I note this contradicts the comment directly above...
• ... and things are right if we take "projective" to mean that we factor out all scalar matrices, i.e., we implicitly work inside an algebraic closure of the ground field. Of course Z(5) does have a square root in GF(25)... But none of the code dealing with project groups seems to do that at the moment (perhaps it should... we ought to write down how the "implicit projective mode" work explicitly, to make this clearer).

Aaaanyway, changing that code to match the initial description of the function does fix this particular error:

diff --git a/gap/projective/classicalnatural.gi b/gap/projective/classicalnatural.gi
index a519fb2..bbcdb02 100644
--- a/gap/projective/classicalnatural.gi
+++ b/gap/projective/classicalnatural.gi
@@ -1528,7 +1528,7 @@ RECOG.IsThisSL2Natural := function(ri,gens,f)
       Info(InfoRecog,4,"SL2: Computing stabiliser chain.");
       S := StabilizerChain(Group(gens));
       Info(InfoRecog,4,"SL2: size is ",Size(S));
-      return Size(S) mod (q*(q-1)*(q+1)) = 0;
+      return Size(S) = (q*(q-1)*(q+1));
   fi;
 
   seenqp1 := false;

Alas, it is unclear to me whether this correct, or just a bandaid that fixes the problem at hand but causes new problems elsewhere?

Perhaps @danielrademacher has insights on this?