Fix RecogniseGroup wrt sorting hints

[ssiccha]

Either the documentation of RecogniseGroup is incorrect, or the function itself is incorrect. See the discussion in #181.

[codecov[bot]]

Codecov Report

Merging #187 into master will increase coverage by 0.33%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #187      +/-   ##
==========================================
+ Coverage   76.07%   76.41%   +0.33%     
==========================================
  Files          37       37              
  Lines       16954    17200     +246     
==========================================
+ Hits        12898    13143     +245     
- Misses       4056     4057       +1     

Impacted Files	Coverage Δ
gap/base/recognition.gd	100.00% <ø> (ø)
gap/base/methsel.gi	94.04% <0.00%> (-1.02%)	:arrow_down:
gap/base/methsel.gd	100.00% <0.00%> (ø)
gap/perm.gi	97.71% <0.00%> (+0.39%)	:arrow_up:
gap/matrix.gi	91.32% <0.00%> (+0.41%)	:arrow_up:
gap/matrix/classical.gi	56.03% <0.00%> (+2.42%)	:arrow_up:
gap/projective.gi	89.17% <0.00%> (+3.45%)	:arrow_up:

[ssiccha]

I've dropped the "Thus you can force..." sentence.

[ssiccha]

@fingolfin What do you think of this?

At the moment, the hint methods are not always called first, despite what the documentation says. This is the responsible line of code. But I ran git blame a little and I have the feeling that the original "sorting the hints and methoddb into rank-descending order" was done on accident in commit 3071737 with title "Add example of successful recognition". I guess that commit should only have touched doc/afterrecog.xml.

[ssiccha]

I just checked which ranks we use to install hints. There are two different situations. Either we choose the rank such that the hint is called before all methods from the respective method database. Or we choose the rank such that the hint is called after the method TrivialGroup but before all other methods from the respective database.

If we want to keep it such that TrivialGroup can be called first, then we should go with the current form of the code, that is concatenating hints and the database and then sorting. Then the documentation would need to be updated.

[fingolfin]

I think the new methods should be prepended and then all sorted in (possibly via StableSort, to ensure that in case of a tie in rank, the hint is first; but probably even better would be to have a test which checks that all ranks are different and if they are not, show an error).

The reason is this: if I want all my hints to be sorted first, I can trivially achieve this by using suitable ranks. But if I want my hints to be interspersed with some existing methods, I cannot do this anymore if we just blindly prepend, unless I am copy&pasting existing methods into the table.

And then we might as well drop those numeric hints completely and just require that the methods tables are set up once at the start with everything in the desired order... To be clear I don't think we should do that: while ranks are somewhat arbitrary, at least they give us a semiclean way to insert extra methods in multiple places, i.e., they help with modularity.