gap-packages / recog

The GAP package recog to collect methods for constructive recognition
https://gap-packages.github.io/recog/
GNU General Public License v3.0
6 stars 14 forks source link

Error in SLPforElementFuncsProjective.PSL2 #317

Open fingolfin opened 2 years ago

fingolfin commented 2 years ago

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 commented 2 years ago

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:

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?