search

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

Get rid of folder gap/matrix/ ? #244

Open ssiccha opened 3 years ago

ssiccha commented 3 years ago

I understand the purpose of gap/matrix/ to contain all the code that handles "standard" i.e. non-projective matrix groups. But I have the impression that it only or mostly contains code, which recog calls with projective inputs anyways.

The files matimpr.gi, classical.g?, and ppd.g? contain code which are directly or, in case of classical.g?, indirectly added to the FindHomDbProjective database.

I'm not sure about slconstr.gi, maybe you @fingolfin or @danielrademacher know more about this?

I think a problem we have due to the files above living in gap/matrix/ but being called with projective inputs is, that for example gap/matrix/classical.gi use code which roughly looke like IsOne(Comm(x,y)). This won't work correctly for projective inputs.

The same happens in slconstr.gi, but as I said, I don't know how the code living there is called and whether that's a problem.

ssiccha commented 3 years ago

Ah, and if we really were to switch to isone(ri)(Comm(x,y)) we could directly use arecommuting which was introduced in #243 (which is now called docommute).

fingolfin commented 3 years ago

Best would be to implemented proper projective matrices (see issue #82) then all these issues would be moot....

ssiccha commented 3 years ago

It would definitely solve the issues with respect to using isone vs IsOne.

But I checked the locations where methods are added to FindHomDbMatrix, which is only in the file gap/matrix.gi. There, only methods defined in gap/matrix.gi itself are added. Which means that the code in gap/matrix/ must either be unused, or included into the projective db, or be called directly from other functions.

So IMO those functions/files that are able to handle projective input and are only called with projective input IMO should live in the gap/projective/ folder. Them staying in the gap/matrix/ folder IMO is confusing for someone trying to understand how recog works. I have the feeling that all the code in gap/matrix/ fits into that category, because those algorithms are always called in a projective setting.

An exception might at the moment be the code for the classical groups, because when we call that in FindHomMethodsProjective.ClassicalNatural we make sure to use only representatives with determinant 1. But also there, @aniemeyer and I are working on making that code work with projective inputs too.

fingolfin commented 3 years ago

All in all, the current directory structure is just a crude first approximation; in the past, all files were in one place. I moved things into subdirs based on quick checks, so I am not surprised if I screwed up some of it. Good that you are looking into it!

However, before you move anything around, I suggest that you verify your various claims by inserting assertions into the various methods that trigger if the input is (not) projective. Or if you find that something is used for both projective and non-projective inputs, add a comment stating so and citing examples that trigger this.

danielrademacher commented 3 years ago

@ssiccha I haven't looked at the file yet, but after reading the contents a bit, I would suspect that a lot of it could be tests from Akos and Max Neunhöffer. In the file, groups that are isomorphic to SL(n, q) for n>= 4, SL(4, q) and SL(2, q) are specially considered and differentiated in some functions. That goes in the direction of what came out in the end. I am currently working through some files in the recog package and have added this file to my list. I may be able to find out more.

@fingolfin I just read Issue #82. Would the implementation of a projective matrix object be interesting for the proposed coding sprint/GAP Days or is that too time-consuming?

fingolfin commented 3 years ago

@danielrademacher I think it would be doable. Well, depending on what one exactly goes for ;-). I'll comment on issue #82

danielrademacher commented 3 years ago

@ssiccha I think the file 'slconstr.gi' contains an algorithm described in 'Black Box Classical Groups (Memoirs of the American Mathematical Society)' by Kantor and Seress. Unfortunately I don't have the book to verify that, but I think this should be correct.

Maybe its similar to "On constructive recognition of a black box PSL( d , q )" by Kantor and Brooksbank.