Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 24 additions & 17 deletions benchmark/matobj/bench-submat.g
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ TestExtractingSubmatrix := function(m)
rows := [2..NrRows(m)-2];
cols := [3..NrCols(m)-1];

PrintHeadline("m{}{}");
MyBench(function()
local u, x;
for u in [1..QuoInt(100000,NrRows(m)*NrCols(m))] do
x:=m{rows}{cols};
od;
end);
if not IsPlistMatrixRep(m) then
PrintHeadline("m{}{}");
MyBench(function()
local u, x;
for u in [1..QuoInt(100000,NrRows(m)*NrCols(m))] do
x:=m{rows}{cols};
od;
end);
fi;

PrintHeadline("ExtractSubMatrix");
MyBench(function()
Expand All @@ -32,13 +34,15 @@ TestCopyingSubmatrix := function(m)
rows := [2..NrRows(m)-2];
cols := [3..NrCols(m)-1];

PrintHeadline("m{}{}:=n{}{}");
MyBench(function()
local u;
for u in [1..QuoInt(100000,NrRows(m)*NrCols(m))] do
x{rows}{cols}:=m{rows}{cols};
od;
end);
if not IsPlistMatrixRep(m) then
PrintHeadline("m{}{}:=n{}{}");
MyBench(function()
local u;
for u in [1..QuoInt(100000,NrRows(m)*NrCols(m))] do
x{rows}{cols}:=m{rows}{cols};
od;
end);
fi;

PrintHeadline("CopySubMatrix");
MyBench(function()
Expand All @@ -62,18 +66,21 @@ for dim in [10, 100] do
PrintBoxed(Concatenation("Now testing in dimension ", String(dim)));

m:=IdentityMat(dim);;
RunMatTest("integer matrix", m);
RunMatTest("integer matrix: list of lists", m);

m:=IdentityMatrix(Integers, dim);;
RunMatTest("integer matrix: IsPlistMatrixRep", m);

m:=IdentityMat(dim,GF(2));;
RunMatTest("GF(2) rowlist", m);

m:=IdentityMat(dim,GF(2));; ConvertToMatrixRep(m);;
m:=IdentityMatrix(GF(2), dim);;
RunMatTest("GF(2) compressed matrix", m);

m:=IdentityMat(dim,GF(7));;
RunMatTest("GF(7) rowlist", m);

m:=IdentityMat(dim,GF(7));; ConvertToMatrixRep(m);;
m:=IdentityMatrix(GF(8), dim);;
RunMatTest("GF(7) compressed matrix", m);

# TODO: also add cvec matrices
Expand Down
81 changes: 81 additions & 0 deletions benchmark/matobj/bench-subvec.g
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
ReadGapRoot("benchmark/matobj/bench.g");


TestExtractingSubvector := function(v)
local cols;

cols := [3..Length(v)-1];

if not IsPlistVectorRep(v) then
PrintHeadline("v{}");
MyBench(function()
local u, x;
for u in [1..QuoInt(100000, Length(v))] do
x := v{cols};
od;
end);
fi;

PrintHeadline("ExtractSubVector");
MyBench(function()
local u, x;
for u in [1..QuoInt(100000, Length(v))] do
x := ExtractSubVector(v, cols);
od;
end);
end;

TestCopyingSubvector := function(v)
local x, cols;

x := ShallowCopy(v);
cols := [3..Length(v)-1];

if not IsPlistVectorRep(v) then
PrintHeadline("x{}:=v{}");
MyBench(function()
local u;
for u in [1..QuoInt(100000, Length(v))] do
x{cols} := v{cols};
od;
end);
fi;

PrintHeadline("CopySubVector");
MyBench(function()
local u;
for u in [1..QuoInt(100000, Length(v))] do
CopySubVector(v, x, cols, cols);
od;
end);
end;

RunVecTest := function(desc, v)
Print("\n");
PrintBoxed(Concatenation("Testing subvector extraction for ", desc));
TestExtractingSubvector(v);
Print(TextAttr.2, "...now testing subvector copying...\n", TextAttr.reset);
TestCopyingSubvector(v);
end;

for dim in [10, 100] do
PrintBoxed(Concatenation("Now testing in dimension ", String(dim)));

v := [1..dim] * 0;;
RunVecTest("integer vector: plain list", v);

v := Vector(IsPlistVectorRep, Integers, [1..dim]);;
RunVecTest("integer vector: IsPlistVectorRep", v);

v := [1..dim] * One(GF(2));;
RunVecTest("GF(2) row vector", v);

v := NewVector(IsGF2VectorRep, GF(2), [1..dim] * One(GF(2)));;
RunVecTest("GF(2) compressed vector", v);

v := [1..dim] * One(GF(7));;
RunVecTest("GF(7) row vector", v);

v := NewVector(Is8BitVectorRep, GF(7), [1..dim] * One(GF(7)));;
RunVecTest("GF(7) 8-bit vector", v);
od;
31 changes: 14 additions & 17 deletions lib/matobj.gi
Original file line number Diff line number Diff line change
Expand Up @@ -749,30 +749,20 @@ InstallMethod( \{\},
return Vector( Unpack( v ){ poss }, v );
end );

InstallMethod( ExtractSubVector,
"generic method for a vector object and a list",
[ IsVectorObj, IsList ],
InstallOtherMethod( ExtractSubVector,
"generic method for a row vector or vector object and a list",
[ IsRowVectorOrVectorObj, IsList ],
{ v, l } -> v{ l } );

InstallMethod( ExtractSubMatrix,
"generic method for a matrix object and two lists",
[ IsMatrixObj, IsList, IsList ],
{ M, rowpos, colpos } -> Matrix( Unpack( M ){ rowpos }{ colpos }, M ) );

# Hack from recog package
InstallOtherMethod( ExtractSubMatrix, "hack: for lists of compressed vectors",
[ IsList, IsList, IsList ],
function( m, poss1, poss2 )
local i,n;
n := [];
for i in poss1 do
Add(n,ShallowCopy(m[i]{poss2}));
od;
if IsFFE(m[1,1]) then
ConvertToMatrixRep(n);
fi;
return n;
end );
InstallOtherMethod( ExtractSubMatrix,
"generic method for a matrix and two lists",
[ IsMatrix, IsList, IsList ],
{ M, rowpos, colpos } -> M{ rowpos }{ colpos } );

InstallMethod( CopySubVector,
"generic method for row vectors and vector objects",
Expand All @@ -788,6 +778,13 @@ InstallMethod( CopySubVector,
od;
end );

InstallMethod( CopySubVector,
"generic method for row vectors",
[ IsRowVector, IsRowVector and IsMutable, IsList, IsList ],
function(src, dst, scols, dcols)
dst{dcols} := src{scols};
end );


#############################################################################
##
Expand Down
17 changes: 11 additions & 6 deletions lib/matobj2.gd
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,8 @@ DeclareGlobalFunction( "ConcatenationOfVectors" );
## </ManSection>
## <#/GAPDoc>
##
DeclareOperation( "ExtractSubVector", [ IsVectorObj, IsList ] );
DeclareOperationKernel( "ExtractSubVector",
[ IsRowVectorOrVectorObj, IsList ], EXTRACT_SUB_VECTOR );


#############################################################################
Expand Down Expand Up @@ -896,9 +897,9 @@ DeclareOperation( "Randomize", [ IsRandomSource, IsMatrixOrMatrixObj and IsMutab
## </ManSection>
## <#/GAPDoc>
##
DeclareOperation( "CopySubVector",
DeclareOperationKernel( "CopySubVector",
[ IsRowVectorOrVectorObj, IsRowVectorOrVectorObj and IsMutable,
IsList, IsList ] );
IsList, IsList ], COPY_SUB_VECTOR );


#############################################################################
Expand Down Expand Up @@ -956,7 +957,8 @@ DeclareOperation( "DistanceOfVectors", [ IsVectorObj, IsVectorObj ] );
## </ManSection>
## <#/GAPDoc>
##
DeclareOperation( "ExtractSubMatrix", [ IsMatrixOrMatrixObj, IsList, IsList ] );
DeclareOperationKernel( "ExtractSubMatrix",
[ IsMatrixOrMatrixObj, IsList, IsList ], EXTRACT_SUB_MATRIX );


#############################################################################
Expand Down Expand Up @@ -1005,8 +1007,11 @@ DeclareOperation( "MutableCopyMatrix", [ IsMatrixOrMatrixObj ] );
## </ManSection>
## <#/GAPDoc>
##
DeclareOperation( "CopySubMatrix",
[ IsMatrixOrMatrixObj, IsMatrixOrMatrixObj, IsList, IsList, IsList, IsList ] );
DeclareOperationKernel( "CopySubMatrix",
[ IsMatrixOrMatrixObj, IsMatrixOrMatrixObj, IsList, IsList, IsList, IsList ],
COPY_SUB_MATRIX );
#T We intentionally keep the second argument declaration broad, mirroring
#T the pre-existing operation declaration for compatibility with packages.


#############################################################################
Expand Down
100 changes: 100 additions & 0 deletions src/lists.c
Original file line number Diff line number Diff line change
Expand Up @@ -1611,6 +1611,99 @@ void AsssListLevel (
}


/****************************************************************************
**
*F FuncEXTRACT_SUB_VECTOR( <self>, <vec>, <poss> ) . . `EXTRACT_SUB_VECTOR'
*/
static Obj ExtractSubVectorOper;

static Obj FuncEXTRACT_SUB_VECTOR(Obj self, Obj vec, Obj poss)
{
if (IS_PLIST(vec)) {
CheckIsPossList("List Elements", poss);
return ELMS_LIST(vec, poss);
}

return DoOperation2Args(ExtractSubVectorOper, vec, poss);
}


/****************************************************************************
**
*F FuncCOPY_SUB_VECTOR( <self>, <src>, <dst>, <scols>, <dcols> )
*/
static Obj CopySubVectorOper;

static Obj FuncCOPY_SUB_VECTOR(
Obj self, Obj src, Obj dst, Obj scols, Obj dcols)
{
if (IS_PLIST(src) && IS_PLIST(dst)) {
Obj rhss;

CheckIsPossList("List Assignments", scols);
CheckIsPossList("List Assignments", dcols);
rhss = ELMS_LIST(src, scols);
AsssListCheck(dst, dcols, rhss);
return 0;
}

return DoOperation4Args(CopySubVectorOper, src, dst, scols, dcols);
}


/****************************************************************************
**
*F FuncEXTRACT_SUB_MATRIX( <self>, <mat>, <rows>, <cols> )
*/
static Obj ExtractSubMatrixOper;

static Obj FuncEXTRACT_SUB_MATRIX(Obj self, Obj mat, Obj rows, Obj cols)
{
if (IS_PLIST(mat)) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The AI originally had this using IS_LIST, which would mean it also handles the IsGF2MatrixRep case. But it would also prevent us form installing better methods for these cases. Plus in all other similar cases, we checked for IS_PLIST.

That's why I changed this to IS_PLIST here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps this comment should go to the code not just to the pull request?

Obj submat;

CheckIsPossList("List Elements", rows);
CheckIsPossList("List Elements", cols);
submat = ELMS_LIST(mat, rows);
ElmsListLevel(submat, cols, 1);
return submat;
}

return DoOperation3Args(ExtractSubMatrixOper, mat, rows, cols);
}


/****************************************************************************
**
*F FuncCOPY_SUB_MATRIX( <self>, <src>, <dst>, <srows>, <drows>, <scols>,
*F <dcols> )
*/
static Obj CopySubMatrixOper;

static Obj FuncCOPY_SUB_MATRIX(
Obj self, Obj src, Obj dst, Obj srows, Obj drows, Obj scols, Obj dcols)
{
if (IS_PLIST(src) && IS_PLIST(dst)) {
Obj srcsub;
Obj dstsub;

CheckIsPossList("List Assignments", srows);
CheckIsPossList("List Assignments", drows);
CheckIsPossList("List Assignments", scols);
CheckIsPossList("List Assignments", dcols);

srcsub = ELMS_LIST(src, srows);
ElmsListLevel(srcsub, scols, 1);
dstsub = ELMS_LIST(dst, drows);
AsssListLevel(dstsub, dcols, srcsub, 1);
return 0;
}

return DoOperation6Args(CopySubMatrixOper, src, dst, srows, drows, scols,
dcols);
}


/****************************************************************************
**
*F PLAIN_LIST(<list>) . . . . . . . . . . . convert a list to a plain list
Expand Down Expand Up @@ -1953,6 +2046,13 @@ static StructGVarOper GVarOpers[] = {

GVAR_OPER_4ARGS(ASS_MAT, mat, row, col, obj, &AssMatOper),
GVAR_OPER_3ARGS(ELM_MAT, mat, row, col, &ElmMatOper),
GVAR_OPER_2ARGS(EXTRACT_SUB_VECTOR, vec, poss, &ExtractSubVectorOper),
GVAR_OPER_4ARGS(COPY_SUB_VECTOR, src, dst, scols, dcols,
&CopySubVectorOper),
GVAR_OPER_3ARGS(EXTRACT_SUB_MATRIX, mat, rows, cols,
&ExtractSubMatrixOper),
GVAR_OPER_6ARGS(COPY_SUB_MATRIX, src, dst, srows, drows, scols, dcols,
&CopySubMatrixOper),

GVAR_OPER_3ARGS(SWAP_MAT_ROWS, mat, row1, row2, &SwapMatRows),
GVAR_OPER_3ARGS(SWAP_MAT_COLS, mat, col1, col2, &SwapMatCols),
Expand Down
3 changes: 2 additions & 1 deletion tst/testinstall/MatrixObj/CopySubVector.tst
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ gap> l4;

#
gap> CopySubVector( l1, l3, [1,2], [1] );
Error, source and destination index lists must be of equal length
Error, List Assignments: <rhss> must have the same length as <poss> (lengths a\
re 2 and 1)

#
gap> STOP_TEST("CopySubVector.tst");
13 changes: 13 additions & 0 deletions tst/testinstall/MatrixObj/ExtractSubMatrix.tst
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
gap> START_TEST("ExtractSubMatrix.tst");
gap> IsBoundGlobal("EXTRACT_SUB_MATRIX");
true
gap> EXTRACT_SUB_MATRIX = ExtractSubMatrix;
true
gap> m1 := [ [ 1, 2, 3 ], [ 4, 5, 6 ] ];
[ [ 1, 2, 3 ], [ 4, 5, 6 ] ]
gap> ExtractSubMatrix( m1, [ 2, 1 ], [ 3, 1 ] );
[ [ 6, 4 ], [ 3, 1 ] ]
gap> m2 := IdentityMatrix( Integers, 4 );;
gap> Unpack( ExtractSubMatrix( m2, [ 2, 4 ], [ 4, 2 ] ) );
[ [ 0, 1 ], [ 1, 0 ] ]
gap> STOP_TEST("ExtractSubMatrix.tst");
2 changes: 2 additions & 0 deletions tst/testinstall/MatrixObj/ExtractSubvector.tst
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
gap> START_TEST("ExtractSubVector.tst");
gap> l1 := [1,2,3,4,5,6];
[ 1, 2, 3, 4, 5, 6 ]
gap> ExtractSubVector( l1, [1,2,4] );
[ 1, 2, 4 ]
gap> v3 := Vector(GF(5), l1*One(GF(5)));
[ Z(5)^0, Z(5), Z(5)^3, Z(5)^2, 0*Z(5), Z(5)^0 ]
gap> ExtractSubVector( v3, [1,2,4] );
Expand Down
Loading