Miscellaneous fixes concerning vector and matrix objects (#6369)

* add checks to `NewVector`, `NewMatrix` for the GF(2) and 8 bit repres.
  in order to catch `fail` results

* add a `String` method for rings `Integers mod n`

* fix the default `OneMutable` method for `IsMatrixObj`
  Test that the input is square, as for list-of-lists matrices.

* minor fixes for `IsPlistVectorRep`
  - check the input list for `IsPlistRep` in `MakeIsPlistVectorRep`,
    let `NewVector` turn the input to `IsPlistRep` if necessary
  - forbid assignments into a `IsPlistVectorRep` vector beyond its length

* fix `SolutionMatDestructive`
  Conceptually, the solution must be in the same representation
  as the given vector.
  (Let us see which other bugs will come to the surface now.)

* fix `TestPositionNonZeroInRow`

Author: ThomasBreuer

hpcgap/lib/vec8bit.gi

@@ -1133,10 +1133,16 @@ InstallMethod( BaseField, "for a compressed 8bit vector",
 InstallTagBasedMethod( NewVector,
   Is8BitVectorRep,
   function( filter, f, l )
-    if ValueOption( "check" ) <> false and not Size(f) in [3..256] then
+    local check, res;
+    check:= ValueOption( "check" ) <> false;
+    if check and not Size(f) in [3..256] then
         Error("Is8BitVectorRep only supports base fields with 3 to 256 elements");
     fi;
-    return CopyToVectorRep(l,Size(f));
+    res:= CopyToVectorRep( l, Size( f ) );
+    if check and res = fail then
+      Error( "cannot copy <l> to 'Is8BitVectorRep'" );
+    fi;
+    return res;
   end );
 
 # This is faster than the default method.
@@ -1170,7 +1176,9 @@ InstallTagBasedMethod( NewMatrix,
     else
       m := List(l,ShallowCopy);
     fi;
-    ConvertToMatrixRep(m,Size(f));
+    if ConvertToMatrixRep( m, Size( f ) ) = fail then
+      Error( "cannot convert <m> to 'Is8BitMatrixRep'" );
+    fi;
     return m;
   end );
 

hpcgap/lib/vecmat.gi

@@ -2555,8 +2555,13 @@ InstallMethod( BaseField, "for a compressed gf2 vector",
 InstallTagBasedMethod( NewVector,
   IsGF2VectorRep,
   function( filter, f, l )
+    local res;
     if Size(f) <> 2 then Error("IsGF2VectorRep only supported over GF(2)"); fi;
-    return CopyToVectorRep(l,2);
+    res:= CopyToVectorRep( l, 2 );
+    if res = fail then
+      Error( "cannot copy <l> to 'IsGF2VectorRep'" );
+    fi;
+    return res;
   end );
 
 InstallTagBasedMethod( NewZeroVector,
@@ -2583,7 +2588,9 @@ InstallTagBasedMethod( NewMatrix,
     else
       m := List(l,ShallowCopy);
     fi;
-    ConvertToMatrixRep(m,2);
+    if ConvertToMatrixRep( m, 2 ) = fail then
+      Error( "cannot convert <m> to 'IsGF2MatrixRep'" );
+    fi;
     return m;
   end );
 

lib/matobj.gi

@@ -1476,7 +1476,14 @@ InstallMethod( ZeroSameMutability,
 
 InstallMethod( OneMutable,
     [ IsMatrixObj ],
-    M -> IdentityMatrix( NumberRows( M ), M ) );
+    function( M )
+    local nrows;
+    nrows:= NrRows( M );
+    if nrows <> NrCols( M ) then
+      Error( "<M> must be square (not ", nrows, " by ", NrCols( M ), ")" );
+    fi;
+    return IdentityMatrix( nrows, M );
+    end );
 
 InstallMethod( OneSameMutability,
     [ IsMatrixOrMatrixObj ],

lib/matobjplist.gi

@@ -88,6 +88,8 @@ BindGlobal( "MakeIsPlistVectorRep",
     if check and ValueOption( "check" ) <> false then
       if not IsSubset( basedomain, list ) then
         Error( "the elements in <list> must lie in <basedomain>" );
+      elif not IsPlistRep( list ) then
+        Error( "<list> must be in 'IsPlistRep'" );
       fi;
     fi;
 
@@ -177,7 +179,7 @@ BindGlobal( "MakeIsPlistMatrixRep",
 InstallTagBasedMethod( NewVector,
   IsPlistVectorRep,
   function( filter, basedomain, list )
-    return MakeIsPlistVectorRep(basedomain, ShallowCopy(list), true);
+    return MakeIsPlistVectorRep(basedomain, PlainListCopy( list ), true);
   end );
 
 InstallTagBasedMethod( NewZeroVector,
@@ -343,6 +345,9 @@ InstallMethod( \[\],
 InstallMethod( \[\]\:\=,
   [ "IsPlistVectorRep", "IsPosInt", "IsObject" ],
   function( v, p, ob )
+    if ValueOption( "check" ) <> false and Length( v![ELSPOS] ) < p then
+      Error( "<p> is out of bounds" );
+    fi;
     v![ELSPOS][p] := ob;
   end );
 
@@ -699,7 +704,7 @@ InstallMethod( Matrix,
     else
         l := [];
     fi;
-    # The result shall be mutable iff 'rows' is mutable.
+    # The result shall be mutable iff 'list' is mutable.
     if not IsMutable( list ) then
       MakeImmutable( l );
     fi;

lib/matrix.gi

@@ -3040,8 +3040,7 @@ InstallOtherMethod( SolutionMatDestructive,
     local i,ncols,sem, vno, z,x, sol;
     ncols := Length(vec);
     z := ZeroOfBaseDomain(mat);
-    sol := ListWithIdenticalEntries(NrRows(mat),z);
-    ConvertToVectorRepNC(sol);
+    sol:= ZeroVector( NrRows( mat ), vec );
     if ncols <> NrCols(mat) then
         Error("SolutionMat: matrix and vector incompatible");
     fi;

lib/vec8bit.gi

@@ -1088,10 +1088,16 @@ InstallMethod( BaseField, "for a compressed 8bit vector",
 InstallTagBasedMethod( NewVector,
   Is8BitVectorRep,
   function( filter, f, l )
-    if ValueOption( "check" ) <> false and not Size(f) in [3..256] then
+    local check, res;
+    check:= ValueOption( "check" ) <> false;
+    if check and not Size(f) in [3..256] then
         Error("Is8BitVectorRep only supports base fields with 3 to 256 elements");
     fi;
-    return CopyToVectorRep(l,Size(f));
+    res:= CopyToVectorRep( l, Size( f ) );
+    if check and res = fail then
+      Error( "cannot copy <l> to 'Is8BitVectorRep'" );
+    fi;
+    return res;
   end );
 
 # This is faster than the default method.
@@ -1125,7 +1131,9 @@ InstallTagBasedMethod( NewMatrix,
     else
       m := List(l,ShallowCopy);
     fi;
-    ConvertToMatrixRep(m,Size(f));
+    if ConvertToMatrixRep( m, Size( f ) ) = fail then
+      Error( "cannot convert <m> to 'Is8BitMatrixRep'" );
+    fi;
     return m;
   end );
 

lib/vecmat.gi

@@ -2553,8 +2553,13 @@ InstallMethod( BaseField, "for a compressed gf2 vector",
 InstallTagBasedMethod( NewVector,
   IsGF2VectorRep,
   function( filter, f, l )
+    local res;
     if Size(f) <> 2 then Error("IsGF2VectorRep only supported over GF(2)"); fi;
-    return CopyToVectorRep(l,2);
+    res:= CopyToVectorRep( l, 2 );
+    if res = fail then
+      Error( "cannot copy <l> to 'IsGF2VectorRep'" );
+    fi;
+    return res;
   end );
 
 InstallTagBasedMethod( NewZeroVector,
@@ -2581,7 +2586,9 @@ InstallTagBasedMethod( NewMatrix,
     else
       m := List(l,ShallowCopy);
     fi;
-    ConvertToMatrixRep(m,2);
+    if ConvertToMatrixRep( m, 2 ) = fail then
+      Error( "cannot convert <m> to 'IsGF2MatrixRep'" );
+    fi;
     return m;
   end );
 

lib/zmodnz.gi

@@ -869,6 +869,7 @@ InstallMethod( TriangulizeMat,
 ##
 #M  ViewObj( <R> )  . . . . . . . . . . . . . . . . method for full ring Z/nZ
 #M  PrintObj( <R> ) . . . . . . . . . . . . . . . . method for full ring Z/nZ
+#M  String( <R> ) . . . . . . . . . . . . . . . . . method for full ring Z/nZ
 ##
 InstallMethod( ViewObj,
     "for full ring Z/nZ",
@@ -884,6 +885,11 @@ InstallMethod( PrintObj,
     Print( "(Integers mod ", Size( obj ), ")" );
     end );
 
+InstallMethod( String,
+    "for full ring Z/nZ",
+    [ IsZmodnZObjNonprimeCollection and IsWholeFamily ], SUM_FLAGS,
+    obj -> Concatenation( "(Integers mod ", String( Size( obj ) ), ")" ) );
+
 
 #############################################################################
 ##

tst/testinstall/MatrixObj/ListOp.tst

@@ -23,4 +23,7 @@ gap> List( v );
 [  ]
 gap> List( v, DegreeFFE );
 [  ]
+gap> v1:= Vector( IsPlistVectorRep, Rationals, [] );;
+gap> v1[1]:= 0;
+Error, <p> is out of bounds
 gap> STOP_TEST("ListOp.tst");

tst/testinstall/MatrixObj/matobjplist.tst

@@ -3,6 +3,8 @@ gap> START_TEST( "matobjplist.tst" );
 
 #
 gap> e:= MakeIsPlistVectorRep( Integers, [], true );;
+gap> MakeIsPlistVectorRep( Integers, [ 1 .. 4 ], true );;
+Error, <list> must be in 'IsPlistRep'
 gap> v:= MakeIsPlistVectorRep( Integers, [ 1 ], true );;
 gap> MakeIsPlistVectorRep( Integers, [ 1/2 ], true );;
 Error, the elements in <list> must lie in <basedomain>