ImmutableMatrix creates MatrixObj for Z/nZ #4797
Conversation
hulpke
added
do not merge
hulpke
removed
the
do not merge
hulpke
force-pushed
the
fixes
branch
3 times, most recently
from
a802b01
to
a7ef741
Compare
hulpke
force-pushed
the
fixes
branch
4 times, most recently
from
79bd931
to
871fb26
Compare
hulpke
force-pushed
the
fixes
branch
2 times, most recently
from
cb965bd
to
83d20fe
Compare
hulpke
force-pushed
the
fixes
branch
3 times, most recently
from
4b4aaed
to
7716404
Compare
| gens,id,true); | ||
|
|
||
| f:=DefaultScalarDomainOfMatrixList(gens); | ||
| gens:=List(Immutable(gens),i->ImmutableMatrix(f,i)); |
There was a problem hiding this comment.
Why do we make an immutable copy of gens before applying List to it? OK, you just copied the code (and that's fine), but I am wondering anyway, and asking in case you have an idea why this is done (if not, please simply ignore this question :-) ).
There was a problem hiding this comment.
I suspect this is the result of a global insertion of ImmutableMatrix in many places without considering in each place whether the local code could be improved subsequently.
lib/vecmat.gi
Outdated
| if IsPlistRep(matrix) and not ForAll(matrix,IsPlistRep) then | ||
| nr:=Length(matrix); | ||
| else | ||
| nr:=NrRows(matrix); | ||
| fi; | ||
|
|
There was a problem hiding this comment.
Why is this necessary? NrRows should work for "old-style" matrices, too
There was a problem hiding this comment.
Actually...
It is needed, because the input could be a mixed format list, for which NrRows will not work.
There was a problem hiding this comment.
Could you elaborate on that a bit? What is a "mixed format list" and where does it occur?
There was a problem hiding this comment.
While merged already, a brief explanation for later:
The function is ImmutableMatrix. It takes somethig that could be construed as a list of vectors, and converts it into a compact format matrix (with the implicit claim that this representation will be efficient to work with).
This could be a list of vectors of different storage/mutability/base ring status (and indeed, this can happen when calling from the MeatAxe).
hulpke
force-pushed
the
fixes
branch
5 times, most recently
from
66c13e5
to
d0bed2b
Compare
if given over large Integers mod n. Subsequent changes in groups code and meataxe to ensure code works. (Added methods for GeneralisedEigenvalues, TriangulizeMat, Check in Echelonization, better field determination in DirectSumMat)
improved binary ops, if one partner is list. Improved view if small. Methiods for `DefaultScalarDomainOfMatrixList`, Hash key, Determinant. Do not use vectorObj for polynomial coeffients
Needed to make groups of matrix objects feasible
New tests for MeatAxe/Group operations using zmodnz matrix objects, Subsequent changed output in tests Don't check for view format of matrices in HPCGAP which for some reason keeps old format.
When one creates a "row space" of vector objects then the base domain of the generators must contain the given left acting domain, otherwise not all scalar multiples of the vectors can be created. Thus it may be necessary to replace the given generators by vectors whose base domain is large enough. (Alternatively, one could signal an error.) As far as I understand, pull request gap-system#4797 had added/changed some code in `lib/vspcrow.gi` that makes it possible to deal with "row vector spaces" whose elements are in `IsVectorObj` (but not lists). A lot of functionality for that is still missing, the current change just makes one test pass that had not failed before because of missing consistency checks.
When one creates a "row space" of vector objects then the base domain of the generators must contain the given left acting domain, otherwise not all scalar multiples of the vectors can be created. Thus it may be necessary to replace the given generators by vectors whose base domain is large enough. (Alternatively, one could signal an error.) As far as I understand, pull request #4797 had added/changed some code in `lib/vspcrow.gi` that makes it possible to deal with "row vector spaces" whose elements are in `IsVectorObj` (but not lists). A lot of functionality for that is still missing, the current change just makes one test pass that had not failed before because of missing consistency checks.
and subsequent changes in group, meataxe, and polynomial code to make it work. This also provides further tests for #4773
The result is a small speedup when working with matrix groups/modules over ZmodnZ rings.
Text for release notes
N/A