From 79bae552e7713009a2b0bb4b18c13a01aa57ad65 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Thu, 9 Apr 2026 22:19:30 +0200 Subject: [PATCH] Allow CopySubVector on plain row vectors, fix CopySubMatrix Broaden CopySubVector to plain row vectors, add some argument validation, and fix the generic CopySubMatrix fallback to require mutability on the destination, not the source. Co-authored-by: Codex --- lib/matobj.gi | 20 +++-- lib/matobj2.gd | 6 +- tst/testinstall/MatrixObj/CopySubMatrix.tst | 84 +++++++++++++++++++++ tst/testinstall/MatrixObj/CopySubVector.tst | 24 ++++++ 4 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 tst/testinstall/MatrixObj/CopySubMatrix.tst diff --git a/lib/matobj.gi b/lib/matobj.gi index bc7a49bc1d..7ee0f91b02 100644 --- a/lib/matobj.gi +++ b/lib/matobj.gi @@ -775,13 +775,13 @@ function( m, poss1, poss2 ) end ); InstallMethod( CopySubVector, - "generic method for vector objects", - [ IsVectorObj, IsVectorObj and IsMutable, IsList, IsList ], + "generic method for row vectors and vector objects", + [ IsRowVectorOrVectorObj, IsRowVectorOrVectorObj and IsMutable, + IsList, IsList ], function(src, dst, scols, dcols) local i; - if not Length( dcols ) = Length( scols ) then - Error( "source and destination index lists must be of equal length" ); - return; + if Length( dcols ) <> Length( scols ) then + ErrorNoReturn( "source and destination index lists must be of equal length" ); fi; for i in [ 1 .. Length( dcols ) ] do dst[dcols[i]] := src[scols[i]]; @@ -1258,10 +1258,18 @@ InstallMethod( MutableCopyMatrix, #M CopySubMatrix( , , , , , ) ## InstallMethod( CopySubMatrix, - [ IsMatrixOrMatrixObj and IsMutable, IsMatrixOrMatrixObj, IsList, IsList, IsList, IsList ], + [ IsMatrixOrMatrixObj, IsMatrixOrMatrixObj and IsMutable, + IsList, IsList, IsList, IsList ], function( src, dst, srcrows, dstrows, srccols, dstcols ) local i, j; + if Length( dstrows ) <> Length( srcrows ) then + ErrorNoReturn( "source and destination row lists must be of equal length" ); + fi; + if Length( dstcols ) <> Length( srccols ) then + ErrorNoReturn( "source and destination column lists must be of equal length" ); + fi; + for i in [ 1 .. Length( srcrows ) ] do for j in [ 1 .. Length( srccols ) ] do dst[ dstrows[i], dstcols[j] ]:= src[ srcrows[i], srccols[j] ]; diff --git a/lib/matobj2.gd b/lib/matobj2.gd index 6b21e5f775..b564dd01e7 100644 --- a/lib/matobj2.gd +++ b/lib/matobj2.gd @@ -875,7 +875,7 @@ DeclareOperation( "Randomize", [ IsRandomSource, IsMatrixOrMatrixObj and IsMutab ## nothing ## ## -## For two vector objects src and dst, +## For two row vectors or vector objects src and dst, ## such that dst is mutable, ## and two lists scols and dcols of positions, ## assigns the entries @@ -897,8 +897,8 @@ DeclareOperation( "Randomize", [ IsRandomSource, IsMatrixOrMatrixObj and IsMutab ## <#/GAPDoc> ## DeclareOperation( "CopySubVector", - [ IsVectorObj, IsVectorObj and IsMutable, IsList, IsList ] ); - + [ IsRowVectorOrVectorObj, IsRowVectorOrVectorObj and IsMutable, + IsList, IsList ] ); ############################################################################# diff --git a/tst/testinstall/MatrixObj/CopySubMatrix.tst b/tst/testinstall/MatrixObj/CopySubMatrix.tst new file mode 100644 index 0000000000..1563c98495 --- /dev/null +++ b/tst/testinstall/MatrixObj/CopySubMatrix.tst @@ -0,0 +1,84 @@ +#@local m1, m2, m3, m4, m5, AsDummyMatrix, DummyMatrixAsList +gap> START_TEST("CopySubMatrix.tst"); +gap> DeclareRepresentation( "IsDummyCopySubMatrixRep6305", +> IsComponentObjectRep and IsAttributeStoringRep and IsMatrixObj, [] ); +gap> InstallMethod( BaseDomain, +> [ IsDummyCopySubMatrixRep6305 ], +> M -> M!.basedomain ); +gap> InstallMethod( NumberRows, +> [ IsDummyCopySubMatrixRep6305 ], +> M -> Length( M!.entries ) ); +gap> InstallMethod( NumberColumns, +> [ IsDummyCopySubMatrixRep6305 ], +> M -> Length( M!.entries[1] ) ); +gap> InstallMethod( \[\], +> [ IsDummyCopySubMatrixRep6305, IsPosInt, IsPosInt ], +> function( M, row, col ) +> return M!.entries[row][col]; +> end ); +gap> InstallMethod( \[\]\:\=, +> [ IsDummyCopySubMatrixRep6305 and IsMutable, IsPosInt, IsPosInt, IsObject ], +> function( M, row, col, obj ) +> M!.entries[row][col] := obj; +> end ); +gap> AsDummyMatrix := function( entries, mutable ) +> local filt, M; +> filt := IsDummyCopySubMatrixRep6305; +> if mutable then +> filt := filt and IsMutable; +> fi; +> M := Objectify( +> NewType( CollectionsFamily( CollectionsFamily( FamilyObj( Zero( Rationals ) ) ) ), +> filt ), +> rec( +> basedomain := Rationals, +> entries := StructuralCopy( entries ) ) ); +> if not mutable then +> MakeImmutable( M!.entries ); +> fi; +> return M; +> end;; +gap> DummyMatrixAsList := function( M ) +> return List( [ 1 .. NumberRows( M ) ], +> i -> List( [ 1 .. NumberColumns( M ) ], j -> M[i, j] ) ); +> end;; + +# +gap> m1 := [ [ 1, 2, 3 ], [ 4, 5, 6 ] ]; +[ [ 1, 2, 3 ], [ 4, 5, 6 ] ] +gap> m2 := [ [ 0, 0, 0, 0 ], [ 0, 0, 0, 0 ], [ 0, 0, 0, 0 ] ]; +[ [ 0, 0, 0, 0 ], [ 0, 0, 0, 0 ], [ 0, 0, 0, 0 ] ] +gap> CopySubMatrix( m1, m2, [ 2, 1 ], [ 1, 3 ], [ 3, 1 ], [ 2, 4 ] ); +gap> m2; +[ [ 0, 6, 0, 4 ], [ 0, 0, 0, 0 ], [ 0, 3, 0, 1 ] ] + +# +gap> m3 := AsDummyMatrix( +> [ [ 0, 0, 0, 0 ], [ 0, 0, 0, 0 ], [ 0, 0, 0, 0 ] ], true );; +gap> CopySubMatrix( m1, m3, [ 1, 2 ], [ 2, 3 ], [ 2, 3 ], [ 4, 1 ] ); +gap> DummyMatrixAsList(m3); +[ [ 0, 0, 0, 0 ], [ 3, 0, 0, 2 ], [ 6, 0, 0, 5 ] ] + +# +gap> m4 := [ [ 0, 0, 0, 0 ], [ 0, 0, 0, 0 ], [ 0, 0, 0, 0 ] ]; +[ [ 0, 0, 0, 0 ], [ 0, 0, 0, 0 ], [ 0, 0, 0, 0 ] ] +gap> CopySubMatrix( m3, m4, [ 3, 2 ], [ 1, 2 ], [ 1, 4 ], [ 3, 2 ] ); +gap> m4; +[ [ 0, 5, 6, 0 ], [ 0, 2, 3, 0 ], [ 0, 0, 0, 0 ] ] + +# +gap> m5 := AsDummyMatrix( m1, false );; +gap> CopySubMatrix( m5, m4, [ 2, 1 ], [ 2, 3 ], [ 2, 1 ], [ 1, 4 ] ); +gap> m4; +[ [ 0, 5, 6, 0 ], [ 5, 2, 3, 4 ], [ 2, 0, 0, 1 ] ] + +# +gap> CopySubMatrix( m5, m4, [ 1, 2 ], [ 1 ], [ 1 ], [ 1 ] ); +Error, source and destination row lists must be of equal length + +# +gap> CopySubMatrix( m5, m4, [ 1 ], [ 1 ], [ 1, 2 ], [ 1 ] ); +Error, source and destination column lists must be of equal length + +# +gap> STOP_TEST("CopySubMatrix.tst"); diff --git a/tst/testinstall/MatrixObj/CopySubVector.tst b/tst/testinstall/MatrixObj/CopySubVector.tst index 688bca39b4..5c702e6089 100644 --- a/tst/testinstall/MatrixObj/CopySubVector.tst +++ b/tst/testinstall/MatrixObj/CopySubVector.tst @@ -1,4 +1,5 @@ #@local l1, v1, l2, v2, v3, v4, x, src, dst, expected, from, to, len, one +#@local l3, l4 gap> START_TEST("CopySubVector.tst"); # @@ -14,6 +15,18 @@ gap> CopySubVector( v2, v1, [1,2,4], [2,4,6] ); gap> Unpack(v1); [ 1, 1, 3, 1, 5, 2 ] +# +gap> l3 := [10,20,30,40,50,60]; +[ 10, 20, 30, 40, 50, 60 ] +gap> CopySubVector( l1, l3, [1,3,5], [6,4,2] ); +gap> l3; +[ 10, 5, 30, 3, 50, 1 ] + +# +gap> CopySubVector( l1, v1, [2,4,6], [1,3,5] ); +gap> Unpack(v1); +[ 2, 1, 4, 1, 6, 2 ] + # 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 ] @@ -57,5 +70,16 @@ gap> for from in [1,2,63,64,65] do > od; > od; +# +gap> l4 := [0,0,0,0,0,0,0,0,0]; +[ 0, 0, 0, 0, 0, 0, 0, 0, 0 ] +gap> CopySubVector( v3, l4, [1,2,3], [2,4,6] ); +gap> l4; +[ 0, Z(5)^0, 0, Z(5), 0, Z(5)^3, 0, 0, 0 ] + +# +gap> CopySubVector( l1, l3, [1,2], [1] ); +Error, source and destination index lists must be of equal length + # gap> STOP_TEST("CopySubVector.tst");