Skip to content

fix a problem with a new feature for matrix groups#6232

Open
ThomasBreuer wants to merge 11 commits intogap-system:masterfrom
ThomasBreuer:TB_Nicomorphism_cache
Open

fix a problem with a new feature for matrix groups#6232
ThomasBreuer wants to merge 11 commits intogap-system:masterfrom
ThomasBreuer:TB_Nicomorphism_cache

Conversation

@ThomasBreuer
Copy link
Copy Markdown
Contributor

@ThomasBreuer ThomasBreuer commented Feb 12, 2026

(resolves #6222)

  • add a description of the cache FULLGLNICOCACHE in the code; it is used only by NicomorphismFFMatGroupOnFullSpace
  • change the keys of FULLGLNICOCACHE: take the field of the matrix entries into account (not only its size), take the ConstructingFilter of the matrices into account
  • in NicomorphismFFMatGroupOnFullSpace, deal with special cases of matrices in IsBlockMatrixRep (which have no ConstructingFilter) and in Is8BitMatrixRep in the situation that the group itself lives over GF(2)
  • add Matrix calls in the function that computes preimages under an action homomorphism with Source a matrix group, in order to support matrix group elements of prescribed kinds of IsMatrixObj.
  • add tests for the new supported situations (most of the changes were actually forced by already available tests)
  • add \in methods for GL and SL groups where the first argument is in IsMatrixObj (such that no nice monomorphism is used in this situation)
  • introduce NormedRowVectors_internal( F, base ) and AsListOfFreeLeftModule_internal( F, base, zero ),
    in order to admit IsVectorObjs in base without supporting the whole vector space machinery for these objects
  • introduce ExternalSet( G ) for matrix groups G, meaning the natural G-set (an undocumented method for permutation groups G was already available)
  • change NicomorphismFFMatGroupOnFullSpace such that we can act with the IsMatrixObj matrices on their IsVectorObj vectors, without unpacking the matrices
  • support IsMatrixObj matrices in the Random methods for GL and SL

(Some of the changes were forced by the fact that functionality for the intended new tests was missing.
In fact the list of known missing functionality is rather growing this way. Issue #6241 was also found just by trying to add tests. Until this problem is fixed, the current pull request deals only partially with the ideas from #6234.)

@ThomasBreuer ThomasBreuer added kind: bug Issues describing general bugs, and PRs fixing them topic: library release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Feb 12, 2026
@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

repeated failures due to problems like
connection error: 12029 fetching http://mirrors.kernel.org/sourceware/cygwin/x86_64/setup.zst.sig

- add a description of the cache `FULLGLNICOCACHE` in the code;
  it is used only by `NicomorphismFFMatGroupOnFullSpace`

- change the keys of `FULLGLNICOCACHE`:
  take the field of the matrix entries into account (not only its size),
  take the `ConstructingFilter` of the matrices into account

- in `NicomorphismFFMatGroupOnFullSpace`, deal with special cases of
  matrices in `IsBlockMatrixRep` (which have no `ConstructingFilter`)
  and in `Is8BitMatrixRep` in the situation that the group itself
  lives over `GF(2)`

- in order to admit the action of `IsMatrixObj` matrices on vectors
  supported by GAP's `Enumerator`s of vector spaces,
  add a `\^` method that `Unpack`s the matrix object
  (Eventually we want to avoid this overhead, but then we need
  `Enumerator`s consisting of vector objects corresponding to the
  matrix objects.)

- add `Matrix` calls in the function that computes preimages under
  an action homomorphism with `Source` a matrix group,
  in order to support matrix group elements of prescribed kinds of
  `IsMatrixObj`.

- add tests for the new supported situations
  (most of the changes were actually forced by already available tests)
- introduce `NormedRowVectors_internal( F, base )`,
  and `AsListOfFreeLeftModule_internal( F, base, zero )`,
  in order to admit `IsVectorObj`s in `base` without supporting
  the whole vector space machinery for these objects

- introduce `ExternalSet( G )` for matrix groups `G`,
  meaning the natural `G`-set
  (an undocumented method for permutation groups `G` was already available)

- change `NicomorphismFFMatGroupOnFullSpace` such that we can act
  with the `IsMatrixObj` matrices on their `IsVectorObj` vectors,
  without unpacking the matrices

- support `IsMatrixObj` matrices in the `Random` methods for GL and SL
@ThomasBreuer ThomasBreuer force-pushed the TB_Nicomorphism_cache branch from 4328b09 to 021e71e Compare March 3, 2026 16:15
@ThomasBreuer ThomasBreuer marked this pull request as ready for review March 4, 2026 09:01
Copy link
Copy Markdown
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I also missed this one :-(

Comment on lines +316 to +317
and ( IsSubset( F, BaseDomain( mat ) ) or
ForAll( Unpack( mat ), row -> IsSubset( F, row ) ) );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment that explains why the "or" case is needed? I.e., because the basedomain may be larger than necessary to represent elements, and we consider explicit conversion an option...

Though... thinking about it... how would one then "coerce" mat into an actual element of G (which can e.g. be multiplied with One(G) ) ? We don't have any tooling for that in GAP so far, do we? Because we don't need to, because all matrices (over a given common base field) are expected to be always "compatible" by some magic...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The real question is what \in for a matrix (object) and a matrix group shall mean:

Can the elements of the group be represented over different base domains?
If yes then arithmetic operations in the group are more expensive, if not then we are free to define a base domain for the group, and we can define \in such that the base domain of the matrix and the group must fit.

Concerning the coercion, I thought that Matrix( filt, R, M ) is intended for that purpose:
M is the given matrix which shall be turned into "a valid group element", R is the base domain of G or of One( G ), and filt is ConstructingFilter( One( G ) ).

lib/grpffmat.gi Outdated
Comment on lines +627 to +631
m:= ZeroMatrix( d, d, m );
repeat
Randomize( rs, m );
det:= DeterminantMat( m );
until not IsZero( det );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we perhaps also have a method for RandomInvertibleMat which takes filt as well?

RandomInvertibleMat( rs, filt, d, F )

Copy link
Copy Markdown
Contributor Author

@ThomasBreuer ThomasBreuer Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently RandomMat and RandomInvertibleMat aren't operations but plain functions.
So the default procedure will be to introduce tag based operations RandomMatrix and RandomInvertibleMatrix for which the new methods (with an optional filter or example matrix as argument) can get installed. (This is analogous to e.g. DiagonalMatrix.)
This way, we avoid the creation of intermediate objects.

Comment on lines +3843 to +3851
InstallTagBasedMethod( RandomInvertibleMatrix,
function( filt, rs, R, m )
local mat, det;

mat:= ZeroMatrix( filt, R, m, m );
repeat
Randomize( rs, mat );
det:= DeterminantMat( mat );
until not IsZero( det );
Copy link
Copy Markdown
Member

@limakzi limakzi Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using random upper-triangular matrix and random lower-triangular matrices such that both $L$ and $U$ satisfy $\det L \not = 0$ and $\det U \not = 0$.
Then, random $A = LU$?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this yield a uniform distribution?

Due to new examples in the chapter about matrices,
the number of `Random` calls has increased,
thus it is not surprising that some outputs involving
such calls have changed.
@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

ThomasBreuer commented Mar 25, 2026

Two kinds of test failures:

  • Manual examples from the chapter about matrices must be adjusted; this is not surprising since this chapter contains now more examples using Random, so outputs involving Random calls likely change.
  • The Semigroups package provides already an operation RandomMatrix, which is not compatible with the RandomMatrix proposed for the GAP library. If Semigroups is loaded, the behaviour changes. I have to think about a solution.

Update on the second kind of failures:
The Semigroups package defines that RandomMatrix( R, n, k ) returns a square matrix with n rows and columns, and rank k, with random entries in R.
The current pull request wants to define that RandomMatrix( R, m, n ) returns a (perhaps not square) matrix with m rows and n columns, with entries in R.
These two definitions are clearly not compatible.
(The method from Semigroups has a higher rank, that's why we see the changed behaviour in the tests for the GAP library.)

@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

Another point:

Currently the determinant of a matrix computed with RandomInvertibleMatrix gets computed in the construction, but the result does not store this attribute.

The Random method for natural SL groups (currently the only customer of RandomInvertibleMatrix) needs the determinant, thus it gets computed again. Would an interface make sense that admits returning also the determinant?

james-d-mitchell added a commit to james-d-mitchell/Semigroups that referenced this pull request Mar 30, 2026
These function conflict with forthcoming changes in GAP, and since they
are undocumented, I'm just removing them in this commit.

See:

gap-system/gap#6232
semigroups#1158
james-d-mitchell added a commit to semigroups/Semigroups that referenced this pull request Mar 30, 2026
These function conflict with forthcoming changes in GAP, and since they
are undocumented, I'm just removing them in this commit.

See:

gap-system/gap#6232
#1158
james-d-mitchell added a commit to james-d-mitchell/Semigroups that referenced this pull request Mar 31, 2026
These function conflict with forthcoming changes in GAP, and since they
are undocumented, I'm just removing them in this commit.

See:

gap-system/gap#6232
semigroups#1158
james-d-mitchell added a commit to semigroups/Semigroups that referenced this pull request Mar 31, 2026
These function conflict with forthcoming changes in GAP, and since they
are undocumented, I'm just removing them in this commit.

See:

gap-system/gap#6232
#1158
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 release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

problem with a new feature for matrix groups

3 participants