gap-system / gap

Main development repository for GAP - Groups, Algorithms, Programming, a System for Computational Discrete Algebra
https://www.gap-system.org
GNU General Public License v2.0
815 stars 161 forks source link

Extend meataxe to be able to detect invariant forms of reducible modules (just one possible form is returned) #5803

Open fingolfin opened 2 months ago

hulpke commented 2 months ago

A main change seems a change from IsomorphismIrred to IsomorphismModules. What I want to make sure is that there will not be a chance of running in infinite recursion.

I have not looked again into the code of these two functions, but the former is a straightforward spinning algorithm, while the second is a rather elaborate calculation that uses a lot of other MeatAxe routines. This means that with this change, InvariantXXXForm cannot be used within routines that might get called from IsomorphismModules. This means that the tests need to cover a variety of module structures (direct sums of multiple isomorphic and non-isomorphic indecomposable components).

fingolfin commented 1 month ago

Calls to MTX.Invariant*Form in GAP:

$ git grep -P -l '\.Invariant.*Form' grp lib
lib/meataxe.gi

So nothing outside of lib/meataxe.gi calls this. Looking closer at those calls in lib/meataxe.gi:

So all in all I don't see how this could cause a problem?

fingolfin commented 2 days ago

@ThomasBreuer @hulpke @jandebeule any thoughts on this PR resp. my question above?

ThomasBreuer commented 2 days ago

No, I do not understand why this scaling happens, and which code may rely on it.

If we decide that this piece of code should get removed then also the line # Replace iso by a scalar multiple to get iso twisted symmetric above that code should get removed (and perhaps should now get another # in front of it).

hulpke commented 2 days ago

This got introduced after 4.B.1 and before 4.4. I have no recall of implementing/committing it, but would not be surprised if I actually did the commit in the past, possibly with code from someone else (as the code formatting, and the long error message do not look as if I wrote it). Alas this is amongst the commits that were deliberately not carried over when we moved to git.

fingolfin commented 2 days ago

I have a private git repository with the full commit history, and tracked the origin of the comment "Replace iso by a scalar multiple" to a commit made 2003-01-06 10:05:29 by @stevelinton with commit message "Derek Holt's code for Invariant forms."

So perhaps I should ask Derek about it...