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

list assignment for matrix objects #4533

Open ThomasBreuer opened 3 years ago

ThomasBreuer commented 3 years ago

The following happens in GAP 4.11.1 as well as in the master branch.

gap> m:= NewZeroMatrix( IsPlistMatrixRep, Integers, 2, 3 );
<2x3-matrix over Integers>
gap> m{[1,2]}:= m{[2,1]};
Error, List Assignments: <rhss> must be a dense list (not a positional object)
[...]

The point is that the operation {}:= (ASSS_LIST) calls AsssListCheck, and this calls RequireDenseList for the third argument (the right hand side of the assignment). However, the matrix objects in question aren't lists, in particular they are not dense lists. We want {}:= for manipulation matrix objects that aren't lists, thus the checks must be changed.

fingolfin commented 3 years ago

Really? I thought the agreement was the opposite, i.e., that {} is only for lists. After all, it can not be efficiently implemented for general matrix objects...

See #4356 for proposed syntax extension that might make sense for matrix objects.

ThomasBreuer commented 3 years ago

O.k., it is more complicated.

The problem in #4517 is about IsRowListMatrix, and currently {}:= is defined for this kind of matrix objects. Thus the question is what we want: forbid {}:= for IsRowListMatrix (and explain this, analogous to the situation with IsVectorObj, in the section about element access) or change the kernel code that expects dense lists on the right hand side.

fingolfin commented 3 years ago

Discussed this with Thomas today. One thing I wasn't aware is that IsRowListMatrix does not imply IsList (this is useful in so far as we may not want to require IsRowListMatrix to really implement the full API for a list type (whatever that means exactly).

But the GAP kernel currently has some checks in the code implementing {} resp. {}:= which checks that the right hand side is a dense list. We check this in a bunch of places in the kernel, both with assertions (e.g. in ASSS_LIST) and explicit checks like CheckIsDenseList("List Assignments", "rhss", objs); in AsssListDefault. We could relax those, but then we also need to go through several kernel functions related to these operations, and carefully change ELMW_LIST invocations to ELM_LIST, to deal with "holes" in the lists....

A maybe easier approach would be change the checks to test for "IsDenseList OR IsRowListMatrix" (here I am assuming that any matrix with IsRowListMatrix(matrix) = true is guaranteed to behave like a dense list in a few key aspects; e.g. matrix[i] is bound for all i in [1 .. Length(matrix)].