Skip to content

updates for vector and matrix objects over residue class rings and large finite fields#6342

Open
ThomasBreuer wants to merge 13 commits intogap-system:masterfrom
ThomasBreuer:TB_matobjnz
Open

updates for vector and matrix objects over residue class rings and large finite fields#6342
ThomasBreuer wants to merge 13 commits intogap-system:masterfrom
ThomasBreuer:TB_matobjnz

Conversation

@ThomasBreuer
Copy link
Copy Markdown
Contributor

The main part of this pull request is to rewrite the code for IsZmodnZVectorRep, IsZmodnZMatrixRep according to the current rules and recommendations for vector and matrix objects, and to reactivate these filters as the default representations for vector/matrix objects over residue class rings and large prime fields. (Thus this resolves #4988.)

In particular, the internal data of an IsZmodnZMatrixRep object is now a plain list of plain lists of integers, no longer a plain list of IsZmodnZVectorRep objects.
(That is, the code for IsZmodnZMatrixRep is analogous to that for IsGenericMatrixRep (see pull request #6322) not to that for IsPlistMatrixRep. However, IsZmodnZVectorRep is analogous to IsPlistVectorRep.)

Besides that, this pull request proposes some fixes for other objects:

  • add checks to NewVector, NewMatrix for the GF(2) and 8 bit representations, in order to catch fail results,
  • add a String method for the rings Integers mod n,
  • fix the default OneMutable method for IsMatrixObj (test that the input is square, as for list-of-lists matrices),
  • check the input list for IsPlistRep in MakeIsPlistVectorRep, and let NewVector turn the input to IsPlistRep if necessary (in fact, it had been possible to cause a segmentation fault by calling AddRowVector with two input vectors created from ranges),
  • forbid assignments into a IsPlistVectorRep vector beyond its length,
  • add a paragraph about implementing new matrix objects: recommend calling Objectify only in one helper function.

Concerning the documentation, there is now a description of the main differences between IsZmodnZVectorRep/IsZmodnZMatrixRep and other representations.
This is quite technical, currently it is not part of the manual.
Perhaps somebody has an idea how to make such descriptions of matrix object types available in general.

The documentation of the two filters now contains a description for which base domains these filters are chosen as the defaults by Vector and Matrix, respectively.
This description could be simplified if the functions DefaultVectorRepForBaseDomain and DefaultMatrixRepForBaseDomain would become documented.
(Perhaps we should think about an "extendible" mechanism, for example via lists of functions, such that adding new types of vector or matrix objects does not require changes in global GAP functions.)

Two open questions (perhaps @hulpke can help answering them):

  • The old code supported binary operations between IsZmodnZVectorRep objects and plain lists of IsZmodnZObj objects, the same for matrices. I have kept this code, but do we really want to have it? If yes then several methods are missing, and more checks of the arguments should be added.
  • The code contains a MinimalPolynomial and CharacteristicPolynomialMatrixNC methods that are installed without explicit requirements for IsZmodnZMatrixRep. Are they intended as generic methods for matrix objects?

Recommend calling `Objectify` only in one helper function.
Test that the input is square, as for list-of-lists matrices.
- check the input list for `IsPlistRep` in `MakeIsPlistVectorRep`,
  let `NewVector` turn the input to `IsPlistRep` if necessary
- forbid assignments into a `IsPlistVectorRep` vector beyond its length
and reactivate them as the default representations for vector/matrix objects
over residue class rings and large prime fields
@ThomasBreuer ThomasBreuer added kind: bug Issues describing general bugs, and PRs fixing them kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: library release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Apr 23, 2026
Conceptually, the solution must be in the same representation
as the given vector.
(Let us see which other bugs will come to the surface now.)
Use the same format as for `IsPlistVectorRep`, `IsPlistMatrixRep`.
@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

The failures in the first attempt had various reasons.

  • A bug in the helper function TestPositionNonZeroInRow was discovered by the fixes for the GF(2) and 8 bit representations.
  • The output produced by the ViewObj methods for IsZmodnZVectorRep and IsZmodnZMatrixRep was different from the output for IsPlistVectorRep and IsPlistMatrixRep, and changing the default representation for the objects in some of the test examples caused the differences. Now I have changed the code of the ViewObj methods such that the old test outputs agree, and I have adjusted the newly added tests. As a consequence, the different ViewObj output for small and large vectors/matrices over residue class rings has now disappeared.
  • I had removed an AddRowVector method for IsPlistRep and IsPlistVectorRep (!) from the file matobjnz.gi (!) because I thought that this was a relic from rewriting the code from matobjplist.gi. Apparently this was not the case, the removed method had been introduced as a workaround for an error in some Dixon-Schneider computations. However, the reason for this error was a bug in SolutionMatDestructive. Thus I have fixed this bug, and did not introduce the workaround again. (Let us see whether other bugs will be discovered due to this fix.)

@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

Essentially only one failure in the second attempt, but this is not easy to fix.
Some functionality is currently available only for row-list matrices (SemiEchelonMatTransformation, TriangilizeMat).
More seriously, the code in lib/ctblgrp.gi (and likely also elsewhere) assumes in many places that one can directly access the rows of the matrices in question.

@hulpke
Copy link
Copy Markdown
Contributor

hulpke commented Apr 24, 2026

More seriously, the code in lib/ctblgrp.gi (and likely also elsewhere) assumes in many places that one can directly access the rows of the matrices in question.

Happy to look at this -- what call should be used to extract a matrix row in vector (or list) form ?

@hulpke
Copy link
Copy Markdown
Contributor

hulpke commented Apr 24, 2026

Two open questions (perhaps @hulpke can help answering them):

  • The old code supported binary operations between IsZmodnZVectorRep objects and plain lists of IsZmodnZObj objects, the same for matrices. I have kept this code, but do we really want to have it? If yes then several methods are missing, and more checks of the arguments should be added.

This was intended as a temporary fallback -- there were situations when a vector arose not in the proper format but as a plist, and this otherwise would have started to unravel all objects to go back to plists. What would be needed as a second step (and I never got to) is to instrument this with an error trigger to identify all cases when it is actually needed (and to fix the code that ends up calling it).

  • The code contains a MinimalPolynomial and CharacteristicPolynomialMatrixNC methods that are installed without explicit requirements for IsZmodnZMatrixRep. Are they intended as generic methods for matrix objects?

As far as I recall not. Probably the extra filter was accidentally left out.

@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

Thanks @hulpke.

Happy to look at this

How shall we proceed then?
The current pull request cannot be merged.
One possibility to get this merged without breaking the Dixon-Schneider code would be to take the proposed code for IsZmodnZMatrixRep as non-row list matrices, but to leave the two representations out from DefaultVectorRepForBaseDomain and DefaultMatrixRepForBaseDomain,
such that they are not used as defaults for vectors and matrices over large prime fields.
(For the Dixon-Schneider computations, we are then back to the situation without this pull request, and use IsPlistMatrixRep and IsPlistVectorRep, where the entries are explicit ZmodnZObj entries.)

what call should be used to extract a matrix row in vector (or list) form ?

I think currently we do not have a function for that.
We could introduce RowOfMatrix( M, i ) for that. (A similar question is addressed in #6239.)
The exact definition will be subtle: we want that any change in a vector object v returned by this function will affect also the matrix M, but this is reasonable only if the data of v are stored inside M; this holds for IsZmodnZVectorRep and IsZmodnZMatrixRep, but we cannot guarantee this in general.
(Maybe we need a filter expressing that RowOfMatrix will return a vector such that changing its entries will change the matrix, and changes in the relevant row of the matrix will change the vector.)

However, perhaps the real question behind this is whether functions that take a matrix M and return a list of vectors should better be defined to return a matrix in the case that M is not a row list matrix. An example is SemiEchelonMat.

For example (this is the place where the first error occurs in Irr( MathieuGroup( 24 ) ) with the current pull request),
DxNiceBasis extracts rows from a given matrix, permutes each row, wraps the result in a matrix, triangulizes this matrix, extracts the rows from the result, and permutes each of them. Using ExtractSubMatrix, all this can be expressed just in terms of matrices, without talking about vectors at all -- is there a reason why the final matrix cannot be used as the return value?

This was intended as a temporary fallback [...]

I understand. Concerning an error trigger, a similar issue arises with Unpack: on the one hand, one wants to see where Unpack gets called, in order to fix the code in question, but on the other hand, there are intentional Unpack calls.

Probably the extra filter was accidentally left out.

O.k., I can make the method installations more restrictive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: bug Issues describing general bugs, and PRs fixing them kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: library

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Re-enable ZmodnZ matrix work

2 participants