ImmutableMatrix should not convert between list of lists matrices and matrix objects

[frankluebeck]

This is a new version of pull request "Some fixes concerning matrices and polynomials over large prime fields (related to issue #4901)" (#4935).

See also the discussion in #4935. This fixes the bugs reported in #4901, but also others.

Problems occured because ImmutableMatrix converted some list of lists matrices into matrix objects, in particular when the base ring was of the form Integers mod n. This is not allowed according to the documentation of vector and matrix objects because mixed arithmetic of "classical" vectors and matrices and vector/matrix objects is not defined.

ImmutableMatrix was originally introduced to ensure efficient representations, in particular of matrices over small fields. But note that the changed representations did not change the behaviour of the objects (e.g., a list of small finite field elements can be multiplied with a matrix in Is8BitMatrixRep over that field in both representations, as plain list or as compact vector in Is8BitVectorRep; the results are the same, the latter is only more efficient). Conversions from list of lists matrices into matrix objects are of a different kind; the resulting objects behave differently compared to the original object, e.g., allowed objects for arithmetic operations are different. This sort of conversion should not be done with ImmutableMatrix.

Of course, it may be desirable to create, say, matrix groups or meataxe modules with matrix objects as elements. These should be created with explicit calls of Matrix or Vector with appropriate base rings and in that case all further data, like matrices of invariant forms, must also be stored as matrix objects over the same base ring.

Text for release notes

not needed (this concerns changes which are not in GAP 4.11)

[hulpke]

Basically the question is on the purpose of ImmutableMatrix. My interpretation, since (e.g.) Group or GModuleByMats call it, was that it was to bring matrices into a good representation for working in the algebra spanned by them. One could declare anothe roperation to this purpose (which in many cases would just call ImmutableMatrix.

[frankluebeck]

@hulpke: I can see what you want. But the use of ImmutableMatrix/Vector in the current library is not good enough for conversions of vectors and matrices into incompatible vector/matrix objects. The bugs I'm addressing with this pull request are all of the kind that some but not all objects in some context (e.g., a field extension, a matrix group, a meataxe module) were converted into vector/matrix objects and this made those structures inconsistent and caused errors.

Before allowing ImmutableMatrix to convert lists of lists to matrix objects it is necessary to go through all places in the library and packages and to check that ImmutableMatrix/Vector is really called on all "related" objects. I'm not sure how difficult this would be.

As suggested above it might be easier to write explicit code using Matrix/Vector to create consistent objects (e.g., certain classical groups) where all data are matrix/vector objects.

[fingolfin]

We need to move forward for the GAP 4.12 release, I don't want to leave this in broken.

I suggest the following:

• [x] we merge my PR #4977
• [ ] we update this PR and merge it
• [ ] we release 4.12
• [ ] then we turn back and see how we can enable @hulpke's nice new code in a "safe" way for broader use

Is everyone on board with this?

[frankluebeck]

@fingolfin : The plan sounds sensible.

I think this PR should go into 4.12.

[fingolfin]

@frankluebeck good! I've merged #4977 now, so can you please rebase this PR and adjust it (I think that mainly means removing those InstallTrueMethod invocations)

[frankluebeck]

Updated, rebased on master, and all checks passed ...