diff --git a/benchmark/matobj/bench-submat.g b/benchmark/matobj/bench-submat.g index 0b10bf85b2..b5ee2697de 100644 --- a/benchmark/matobj/bench-submat.g +++ b/benchmark/matobj/bench-submat.g @@ -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() @@ -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() @@ -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 diff --git a/benchmark/matobj/bench-subvec.g b/benchmark/matobj/bench-subvec.g new file mode 100644 index 0000000000..8e2ede2ea9 --- /dev/null +++ b/benchmark/matobj/bench-subvec.g @@ -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; diff --git a/lib/matobj.gi b/lib/matobj.gi index 652dddccad..35ae1ccbe0 100644 --- a/lib/matobj.gi +++ b/lib/matobj.gi @@ -749,9 +749,9 @@ 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, @@ -759,20 +759,10 @@ InstallMethod( ExtractSubMatrix, [ 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", @@ -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 ); + ############################################################################# ## diff --git a/lib/matobj2.gd b/lib/matobj2.gd index c7cd9f0c4a..21f4af9a32 100644 --- a/lib/matobj2.gd +++ b/lib/matobj2.gd @@ -434,7 +434,8 @@ DeclareGlobalFunction( "ConcatenationOfVectors" ); ## ## <#/GAPDoc> ## -DeclareOperation( "ExtractSubVector", [ IsVectorObj, IsList ] ); +DeclareOperationKernel( "ExtractSubVector", + [ IsRowVectorOrVectorObj, IsList ], EXTRACT_SUB_VECTOR ); ############################################################################# @@ -896,9 +897,9 @@ DeclareOperation( "Randomize", [ IsRandomSource, IsMatrixOrMatrixObj and IsMutab ## ## <#/GAPDoc> ## -DeclareOperation( "CopySubVector", +DeclareOperationKernel( "CopySubVector", [ IsRowVectorOrVectorObj, IsRowVectorOrVectorObj and IsMutable, - IsList, IsList ] ); + IsList, IsList ], COPY_SUB_VECTOR ); ############################################################################# @@ -956,7 +957,8 @@ DeclareOperation( "DistanceOfVectors", [ IsVectorObj, IsVectorObj ] ); ## ## <#/GAPDoc> ## -DeclareOperation( "ExtractSubMatrix", [ IsMatrixOrMatrixObj, IsList, IsList ] ); +DeclareOperationKernel( "ExtractSubMatrix", + [ IsMatrixOrMatrixObj, IsList, IsList ], EXTRACT_SUB_MATRIX ); ############################################################################# @@ -1005,8 +1007,11 @@ DeclareOperation( "MutableCopyMatrix", [ IsMatrixOrMatrixObj ] ); ## ## <#/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. ############################################################################# diff --git a/src/lists.c b/src/lists.c index dc329b0966..feb5c1584c 100644 --- a/src/lists.c +++ b/src/lists.c @@ -1611,6 +1611,99 @@ void AsssListLevel ( } +/**************************************************************************** +** +*F FuncEXTRACT_SUB_VECTOR( , , ) . . `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( , , , , ) +*/ +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( , , , ) +*/ +static Obj ExtractSubMatrixOper; + +static Obj FuncEXTRACT_SUB_MATRIX(Obj self, Obj mat, Obj rows, Obj cols) +{ + if (IS_PLIST(mat)) { + 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( , , , , , , +*F ) +*/ +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() . . . . . . . . . . . convert a list to a plain list @@ -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), diff --git a/tst/testinstall/MatrixObj/CopySubVector.tst b/tst/testinstall/MatrixObj/CopySubVector.tst index 5c702e6089..33cea42c90 100644 --- a/tst/testinstall/MatrixObj/CopySubVector.tst +++ b/tst/testinstall/MatrixObj/CopySubVector.tst @@ -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: must have the same length as (lengths a\ +re 2 and 1) # gap> STOP_TEST("CopySubVector.tst"); diff --git a/tst/testinstall/MatrixObj/ExtractSubMatrix.tst b/tst/testinstall/MatrixObj/ExtractSubMatrix.tst new file mode 100644 index 0000000000..35517c5f25 --- /dev/null +++ b/tst/testinstall/MatrixObj/ExtractSubMatrix.tst @@ -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"); diff --git a/tst/testinstall/MatrixObj/ExtractSubvector.tst b/tst/testinstall/MatrixObj/ExtractSubvector.tst index 42af85ac22..81224516ab 100644 --- a/tst/testinstall/MatrixObj/ExtractSubvector.tst +++ b/tst/testinstall/MatrixObj/ExtractSubvector.tst @@ -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] );