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
813 stars 161 forks source link

Regression in `DirectSumMat` #5434

Closed fingolfin closed 1 year ago

fingolfin commented 1 year ago

From a Stack exchange question:

gap> F:=FunctionField(Rationals,["x1","x2","x3","x4"]);AssignGeneratorVariables(F);
FunctionField(...,[ x1, x2, x3, x4 ])
#I  Assigned the global variables [ x1, x2, x3, x4 ]
gap> N:=[[x1,x2],[x3,x4]];U:=DirectSumMat(N,N);
[ [ x1, x2 ], [ x3, x4 ] ]
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsSubset' on 2 arguments
The 1st argument is 'fail' which might point to an earlier problem
The 2nd argument is 'fail' which might point to an earlier problem
 at /Users/mhorn/Projekte/GAP/gap/lib/methsel2.g:250 called from
IsSubset( F, c ) at /Users/mhorn/Projekte/GAP/gap/lib/matrix.gi:4165 called from
<function "DirectSumMat">( <arguments> )
 called from read-eval loop at *stdin*:2
type 'quit;' to quit to outer loop

This works fine in GAP 4.11.1 and older. Before it worked as expected:

gap> F:=FunctionField(Rationals,["x1","x2","x3","x4"]);AssignGeneratorVariables(F);
FunctionField(...,[ x1, x2, x3, x4 ])
#I  Assigned the global variables [ x1, x2, x3, x4 ]
gap> N:=[[x1,x2],[x3,x4]];U:=DirectSumMat(N,N);
[ [ x1, x2 ], [ x3, x4 ] ]
[ [ x1, x2, 0, 0 ], [ x3, x4, 0, 0 ], [ 0, 0, x1, x2 ], [ 0, 0, x3, x4 ] ]

It's also fairly clear what went wrong (basically an attempt to make this function ready for MatrixObj support went wrong), and a fix shouldn't be too hard. There are more inputs that regressed, e.g. "empty" matrices.

In GAP 4.11.1: this works:

gap> DirectSumMat();
[  ]
gap> DirectSumMat([]);
[  ]
gap> DirectSumMat([], []);
[  ]
gap> DirectSumMat([], [], [[1]]);
[ [ 1 ] ]

In current GAP, all of these produces failures.

ThomasBreuer commented 1 year ago

I agree that the abovementioned examples should work. Currently DirectSumMat is undocumented, thus it is not clear what we really want to get. In order to make the function fit into the MatrixObj context, more would be needed: An optional filter (the intended internal representation of the result) or example matrix object (meaning that the result shall have the same internal representation as this matrix object) should be supported, and we must specify whether all given input matrix objects must have the same base domain.