Make RECOG.IsThisSL2Natural more precise

[SoongNoonien]

This is a test to see if a change in #371 makes sense.

[fingolfin]

Thanks for starting this experiment.

So now we know: With this change we get a bunch of errors in tests related to the naming / recognition of $(P)SL(2,q)$:

testing: /tmp/gaproot/pkg/recog/tst/working/veryslow/ClassicalNatural.tst
########> Diff in /tmp/gaproot/pkg/recog/tst/working/veryslow/ClassicalNatural.tst:60
# Input is:
TestRecogGL(2,4);;
# Expected output:
Stamp: ClassicalNatural
# But found:
Stamp: LowIndex
########
########> Diff in /tmp/gaproot/pkg/recog/tst/working/veryslow/ClassicalNatural.tst:86
# Input is:
TestRecogGL(2,5);;
# Expected output:
Stamp: ClassicalNatural
# But found:
Stamp: LowIndex
########
########> Diff in /tmp/gaproot/pkg/recog/tst/working/veryslow/ClassicalNatural.tst:112
# Input is:
TestRecogGL(2,8);;
# Expected output:
Stamp: ClassicalNatural
# But found:
Stamp: StabilizerChainProj
########
########> Diff in /tmp/gaproot/pkg/recog/tst/working/veryslow/ClassicalNatural.tst:138
# Input is:
TestRecogGL(2,9);;
# Expected output:
Stamp: ClassicalNatural
# But found:
Stamp: StabilizerChainProj
########
########> Diff in /tmp/gaproot/pkg/recog/tst/working/veryslow/ClassicalNatural.tst:154
# Input is:
TestRecogGL(10,9);;
# Expected output:
Stamp: ClassicalNatural
# But found:
ALARM: set count to -1 to skip test!
...

So at least with the current code, we need this.

But it may still be that it only papers over a symptom instead of fixing a root cause. Note that the code originally had the stricter = test and I weakened it in 7bd8018

Thing is, the name of this function strongly suggests it is meant to detect the matrix group $SL_2$; but this code and some other comments it is meant to detect $PSL_2$. This discrepency should be resolved, one way or the other.

[fingolfin]

Looking at the (currently) only caller for RECOG.IsThisSL2Natural, we see this:

  # First check whether we are applicable:
  if d = 2 then
      if not RECOG.IsThisSL2Natural(GeneratorsOfGroup(g),f) then
          Info(InfoRecog,2,"ClassicalNatural: Is not PSL_2.");
          return TemporaryFailure; # FIXME: TemporaryFailure here really correct?
      fi;
  else
      classical := RecogniseClassical(g);
      if classical.isSLContained <> true then
          Info(InfoRecog,2,"ClassicalNatural: Is not PSL.");
          return TemporaryFailure; # FIXME: TemporaryFailure here really correct?
      fi;
  fi;

  # Now get rid of nasty determinants:
  gens := ShallowCopy(GeneratorsOfGroup(g));
  changed := false;
  gcd := Gcdex(d,q-1);
  for i in [1..Length(gens)] do
      det := DeterminantMat(gens[i]);
      if not IsOne(det) then
          root := RootFFE(f, det^-1, d);
          if root = fail then
              Info(InfoRecog,2,
                   "ClassicalNatural: determinant cannot be normalized in field.");
              return fail;
          fi;
          gens[i] := gens[i] * root;
          changed := true;
      fi;
  od;

So: we first call RECOG.IsThisSL2Natural or RecogniseClassical, and only if those give a "green light' do we proceed to adjust the matrices to have "nice" determinant.

So one alternate fix (which might allow merging this PR here) could be to interchange these two code blocks, and "normalize" the matrices first.

(This PR should not be merged in its current form, though: at the very least, some of the code comments added in 7bd8018 should be adjusted accordingly, or be removed again)

[SoongNoonien]

Ok, I tried to reorder these two blocks and we'll see if it works now.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.11%. Comparing base (67ae22b) to head (b1e88ef).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #487      +/-   ##
==========================================
- Coverage   88.13%   88.11%   -0.02%     
==========================================
  Files          45       45              
  Lines       19023    19025       +2     
==========================================
- Hits        16765    16764       -1     
- Misses       2258     2261       +3     

Files with missing lines	Coverage Δ
gap/projective/classicalnatural.gi	69.26% <100.00%> (-0.06%)	⬇️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.