Reduce number of arguments of recognition methods

[fingolfin]

Right now recognition methods take two arguments: ri and G. But G is always identical to Grp(ri). Alas, not everyone will know this, e.g. I am re-discovering this every time I start digging into recog again. This can be very confusing ("why are both these groups around? Is there any subtle difference????"). One could solve that by trying to document this, but I think it is much better to just remove the second argument.

---

This is incomplete, a lot more needs to be changed in methods. This can only be partially automated with classical scripts (and I don't trust AI to do this), but it's actually not hard to do. Alas: I'd like to defer this until after merging PR #330 (and perhaps a few others, to minimize conflicts for those).

Resolves #185 (once it is done)

[SoongNoonien]

Apparently, Grp does not exist. @fingolfin do you know what is going on here? Accessing ri.grp directly gives an error as well, because this field seems to be be protected somehow?

[fingolfin]

@SoongNoonien not sure what you mean by "Grp does not exist" ? It is declared in gap/base/recognition.gd:111.

Direct access to "components" of objects in GAP is via ri!.Grp (for example). Think of this as "fields" versus "properties" in Julia: for plain GAP records (= Julia struct, for the purpose of this comparison), the two concepts are the same. But for objects, the fields are "hidden" (unlike julia, were by default getproperty delegates to getfield).

---

Anyway presumably you are getting or seeing an error somehow somewhere, could you clarify? I didn't see any such error in the CI logs (Watch out for syntax errors etc. when loading GAP / the packages).

Although, this PR now has a merge conflict again, so CI tests won't run until that's resolved.

[SoongNoonien]

Anyway presumably you are getting or seeing an error somehow somewhere, could you clarify?

I'm talking about this error: Error, no 1st choice method found for `Grp' on 1 arguments

Currently, I write Grp(ri) which causes this issue. If I understand you correctly this should rather be ri!.Grp?

[TWiedemann]

Grp(ri) should work (and be the preferred technique), I do not know what is going wrong here. Maybe something went wrong earlier.

[fingolfin]

I've resolved the merge conflict.

The recommend way is indeed Grp(ri). If that produces an error, check whether ri is of the right type.

[fingolfin]

gap> ReadPackage("recog", "tst/naming.g");
true
gap> d:=3;; for q in [2, 3] do TestNaming("SL", d, q); od;
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `Grp' on 1 arguments
*[1] Grp( recognise )
   in unknown function
   @ ~/Projekte/GAP/repos/pkg/gap-packages/recog/gap/matrix/classical.gi:695
 [2] f( ri )
   in function "CallRecogMethod"
   @ ~/Projekte/GAP/repos/pkg/gap-packages/recog/gap/base/methods.gi:94
 [3] CallRecogMethod( db[i].method, ri )
   in function "CallMethods"
   @ ~/Projekte/GAP/repos/pkg/gap-packages/recog/gap/base/methsel.gi:89
 [4] CallMethods( ClassicalMethDb, opt.nrrandels, recognise )
   in function "RecogniseClassical"
   @ ~/Projekte/GAP/repos/pkg/gap-packages/recog/gap/matrix/classical.gi:2458
 [5] RecogniseClassical( G )
   in function "TestNaming"
   @ ~/Projekte/GAP/repos/pkg/gap-packages/recog/tst/naming.g:36
...  at *stdin*:2
type 'quit;' to quit to outer loop
brk> recognise;
rec( BE := [  ], ClassicalForms := [  ], E := [  ], E2 := [  ], LB := [  ], LE := [  ],
  LS := [  ], QuadraticForm := false, QuadraticFormType := "unknown", a := 1,
  bc := "unknown", cpol := fail, currentgcd := 3, d := 3, dimsReducible := [  ],
  field := GF(2), g := fail, grp := SL(3,2), hasSpecialEle := false, hint := "unknown",
  hintIsWrong := false, isGeneric := "unknown", isNotAlternating := "unknown",
  isNotExt := "unknown", isNotMathieu := "unknown", isNotPSL := "unknown",
  isOmegaContained := "unknown", isReducible := "unknown", isSLContained := "unknown",
  isSUContained := "unknown", isSpContained := "unknown", isppd := fail,
  kroneckerFactors := "unknown", maybeDual := true, maybeFrobenius := false,
  module := rec( IsOverFiniteField := true, dimension := 3, field := GF(2),
      generators := [ <an immutable 3x3 matrix over GF2>,
          <an immutable 3x3 matrix over GF2> ], isMTXModule := true ), n := 0,
  needBaseChange := false, needDecompose := false, needE2 := false, needForms := true,
  needKroneckerFactors := false, needLB := false, needMeataxe := false,
  needOrders := false, needPOrders := false, needPlusMinus := false, orders := [  ],
  p := 2, plusminus := [  ], porders := [  ], possibleNearlySimple := [  ], q := 2,
  scalars := Group([ <an immutable 2x2 matrix over GF2> ]),
  sq1 := [ <an immutable 2x2 matrix over GF2> ],
  sq2 := [ <an immutable 2x2 matrix over GF2> ] )

So yeah, that's not a recognition method. Indeed, the code in gap/matrix/classical.gi uses the "recognition method" framework for its naming, and works quite a bit different.

But using recognise.grp instead of Grp(recognise) should work fine there

[SoongNoonien]

Ok, this seems to have cleared these errors. I'll work on the other ones and reduce the diff later on.

[codecov]

Codecov Report

❌ Patch coverage is 85.91549% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.04%. Comparing base (23e1197) to head (6b12678).

Files with missing lines	Patch %	Lines
gap/matrix/matimpr.gi	40.00%	3 Missing ⚠️
gap/perm.gi	77.77%	2 Missing ⚠️
gap/generic/slconstr.gi	0.00%	1 Missing ⚠️
gap/matrix/blocks.gi	88.88%	1 Missing ⚠️
gap/matrix/classical.gi	90.90%	1 Missing ⚠️
gap/projective.gi	75.00%	1 Missing ⚠️
gap/projective/almostsimple.gi	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #355      +/-   ##
==========================================
+ Coverage   73.01%   73.04%   +0.03%     
==========================================
  Files          44       44              
  Lines       18370    18414      +44     
==========================================
+ Hits        13412    13451      +39     
- Misses       4958     4963       +5     

Files with missing lines	Coverage Δ
gap/base/methods.gi	92.30% <100.00%> (+0.15%)	⬆️
gap/base/methsel.gd	100.00% <ø> (ø)
gap/base/methsel.gi	91.17% <100.00%> (ø)
gap/base/recognition.gi	70.91% <100.00%> (ø)
gap/generic/FewGensAbelian.gi	93.33% <100.00%> (ø)
gap/generic/KnownNilpotent.gi	97.18% <100.00%> (ø)
gap/generic/SnAnUnknownDegree.gi	90.11% <100.00%> (+0.02%)	⬆️
gap/generic/TrivialGroup.gi	100.00% <100.00%> (ø)
gap/matrix.gi	77.33% <100.00%> (+0.30%)	⬆️
gap/perm/giant.gi	88.99% <100.00%> (+0.02%)	⬆️
... and 15 more

🚀 New features to boost your workflow:

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