Skip to content

add and integrate a SL2 in natural rep. recognition algorithm by Frank Lübeck #389

Open
Till-Eisen wants to merge 13 commits intogap-packages:Include_ConstructiveRecognition_SLfrom
Till-Eisen:Include_ConstructiveRecognition_SL
Open

add and integrate a SL2 in natural rep. recognition algorithm by Frank Lübeck #389
Till-Eisen wants to merge 13 commits intogap-packages:Include_ConstructiveRecognition_SLfrom
Till-Eisen:Include_ConstructiveRecognition_SL

Conversation

@Till-Eisen
Copy link
Collaborator

@Till-Eisen Till-Eisen commented Feb 17, 2026

Added Frank Lübeck's RecogNaturalSL2 algorithm as well as ConRecogNaturalSL2, which calls it and converts the output to the same format as RECOG.RecogniseSL2NaturalEvenChar / RECOG.RecogniseSL2NaturalOddCharUsingBSGS. ConRecogNaturalSL2 retries up to 100 times until the correct generators are found (usually takes very few attempts, i.e. 100 is a very high bound).
Also added test_ConRecogNaturalSL2 for testing: it accepts either a list of prime powers or an empty list — in the latter case a preset list of prime powers is tested automatically.

Note RecogNaturalSL2 does not work for q=2,3,5. In this case the old functions are called.

@codecov
Copy link

codecov bot commented Feb 17, 2026

Codecov Report

❌ Patch coverage is 87.50000% with 28 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (Include_ConstructiveRecognition_SL@5120658). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...projective/constructive_recognition/SL/BaseCase.gi 88.18% 26 Missing ⚠️
gap/projective/constructive_recognition/SL/main.gi 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                          Coverage Diff                          @@
##             Include_ConstructiveRecognition_SL     #389   +/-   ##
=====================================================================
  Coverage                                      ?   65.52%           
=====================================================================
  Files                                         ?       49           
  Lines                                         ?    20671           
  Branches                                      ?        0           
=====================================================================
  Hits                                          ?    13545           
  Misses                                        ?     7126           
  Partials                                      ?        0           
Files with missing lines Coverage Δ
...ojective/constructive_recognition/utils/achieve.gi 85.08% <100.00%> (ø)
gap/projective/constructive_recognition/SL/main.gi 41.91% <0.00%> (ø)
...projective/constructive_recognition/SL/BaseCase.gi 47.76% <88.18%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Till-Eisen Till-Eisen force-pushed the Include_ConstructiveRecognition_SL branch 2 times, most recently from 7abb134 to 84ff4f1 Compare February 17, 2026 10:29
@Till-Eisen Till-Eisen changed the title add and integrate Frank Lübeck SL2 recognition algorithm add and integrate Frank Lübeck SL2 in natural rep. recognition algorithm Feb 17, 2026
@Till-Eisen Till-Eisen changed the title add and integrate Frank Lübeck SL2 in natural rep. recognition algorithm add and integrate a SL2 in natural rep. recognition algorithm by Frank Lübeck Feb 17, 2026
@Till-Eisen Till-Eisen force-pushed the Include_ConstructiveRecognition_SL branch 2 times, most recently from 15c6c86 to 894600f Compare February 17, 2026 12:48
@SoongNoonien
Copy link
Collaborator

I think we should just raise the minimum GAP version to 4.13. The problem with the 4.12 test failure is that the function DLog is apparently not available.

Comment on lines +794 to +798
## test function: compares ConRecogNaturalSL2 against the built-in recognition
## note: does not work for q = 2, 3, 5
## Input: either a list of prime powers or the empty list
## (then a preset list of prime powers is tested).
test_ConRecogNaturalSL2 := function(input)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be turned into "proper" tests.

For example it could be moved into tst/utils.g or some other tst/BLAH.g file (could even be a new one); and a .tst file (either an existing one or a new one) should invoke this helper in a suitable way to ensure some of these tests are run as part of our CI.

Feel free to ask about details.

res_old := RECOG.RecogniseSL2NaturalOddCharUsingBSGS(G,f);
fi;
res := RECOG.ConRecogNaturalSL2(G,f);
## compare all generators after change of basis
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah so this test "only" checks that the old and new method get to the same result.

That maybe changes whether we should add these tests to the permanent tests, as the plan would be to delete RECOG.RecogniseSL2NaturalEvenChar and RECOG.RecogniseSL2NaturalOddCharUsingBSGS at some point.

Also, in the end, the "comparison" matrices can be reproduce much cheaper via

  std := RECOG.MakeSL_StdGens(Characteristic(f),DegreeOverPrimeField(f),2,2);

So you could then compare res.all[i]^res.basi to those and not care about res_old at all.

@Till-Eisen Till-Eisen force-pushed the Include_ConstructiveRecognition_SL branch from eaf9760 to 3407986 Compare March 3, 2026 09:45
Comment on lines 872 to 878
if IsEvenInt(q) then
std := RECOG.RecogniseSL2NaturalEvenChar(gm,f,false);
std := RECOG.ConRecogNaturalSL2(gm,f);
ri!.comment := "PSL2Even";
else
std := RECOG.RecogniseSL2NaturalOddCharUsingBSGS(gm,f);
std := RECOG.ConRecogNaturalSL2(gm,f);
ri!.comment := "PSL2Odd";
fi;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be merged together now, with ri!.comment := "PSL2";

else
resl2 := RECOG.RecogniseSL2NaturalOddCharUsingBSGS(Group(sl2genss),f);
fi;
resl2 := RECOG.ConRecogNaturalSL2(Group(sl2genss),f);
Copy link
Member

@fingolfin fingolfin Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For fun I did a local benchmark:

gap> q:=2;; for i in [1..1000] do RECOG.RecogniseSL2NaturalOddCharUsingBSGS(SL(2,q),GF(q)); od; time;
608
gap> q:=2;; for i in [1..1000] do RECOG.RecogniseSL2NaturalEvenChar(SL(2,q),GF(q),false); od; time;
302

Note that I run the test a 1000 times in a loop to get better "accuracy. So 302 milliseconds for 1000 runs means 0.3 milliseconds per run. Not bad. And the specialized method indeed was faster.

Now you could try to compare this to your new code.

## as RECOG.RecogniseSL2NaturalEvenChar and
## RECOG.RecogniseSL2NaturalOddCharUsingBSGS.
## G must be SL(2,q) generated by 2x2 matrices over GF(q).
RECOG.ConRecogNaturalSL2 := function(G, f)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not yet looked at the content of this function, but I did run some benchmarks and they show that this function adds quite some overhead over Frank's code:

gap> q:=13^2;; for i in [1..1000] do RECOG.RecogniseSL2NaturalOddCharUsingBSGS(SL(2,q),GF(q)); od; time;
2432
gap> q:=13^2;; for i in [1..1000] do RECOG.ConRecogNaturalSL2(SL(2,q),GF(q)); od; time;
1202
gap> q:=13^2;; for i in [1..1000] do RECOG.RecogNaturalSL2(SL(2,q),q); od; time;
814

So it's still twice faster than the old code, but 50% slower than Frank's code.

In even char however it is almost three times slower than the old code, yet Frank's code is three times faster than the old code.

gap> q:=8;; for i in [1..1000] do RECOG.RecogniseSL2NaturalEvenChar(SL(2,q),GF(q),false); od; time;
1404
gap> q:=8;; for i in [1..1000] do RECOG.ConRecogNaturalSL2(SL(2,q),GF(q)); od; time;
4116
gap> q:=8;; for i in [1..1000] do RECOG.RecogNaturalSL2(SL(2,q),q); od; time;
513

true_u1 := [[Z(q)^0,0*Z(q)],[Z(q)^0,Z(q)^0]];
true_u2 := [[Z(q)^0,Z(q)^0],[0*Z(q),Z(q)^0]];
## retry until RecogNaturalSL2 returns generators matching the standard form
## TODO: Perhaps remove this for-loop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes definitely. Why is it even here? Does Frank's code sometimes fail?

[ some time later ]

Aha indeed, for even q, Frank's code is not quite right. Maybe @frankluebeck can suggest a fix. Here's a reproducer:

q:=8;;
G:=SL(2,q);;
res:=RECOG.RecogNaturalSL2(G,q);
nicegens :=List(res[1],a->ResultOfStraightLineProgram(a,GeneratorsOfGroup(G)));
diag := nicegens[1]^res[2];
u1 := nicegens[2]^res[2];
u2 := nicegens[3]^res[2];
Display(diag);
Display(u1);
Display(u2);

If one plugs in an odd q this works nicely, but for even q only sometimes. Indeed, the test code below tries it 1000 timesl for me it worked in just 242 cases (obviously it will fluctuate depending on the RNG seed):

q:=8;;
G:=SL(2,q);;
good:=0;;
for i in [1..1000] do
  res:=RECOG.RecogNaturalSL2(G,q);
  nicegens :=List(res[1],a->ResultOfStraightLineProgram(a,GeneratorsOfGroup(G)));
  diag := nicegens[1]^res[2];
  u1 := nicegens[2]^res[2];
  u2 := nicegens[3]^res[2];
  if IsDiagonalMat(diag) and IsLowerTriangularMat(u1) and IsUpperTriangularMat(u2) then
    good := good + 1;
  fi;
od;

In the meantime, I suggest we keep using RECOG.RecogniseSL2NaturalEvenChar for even q until someone has a fix for Frank's code.

@Till-Eisen Till-Eisen force-pushed the Include_ConstructiveRecognition_SL branch from 74e54f9 to 10a6ec2 Compare March 5, 2026 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants