Improve (New)ZeroMatrix, (New)IdentityMatrix, (New)ZeroVector to better reject invalid input

hpcgap/lib/vec8bit.gi

@@ -1140,34 +1140,31 @@ InstallMethod( BaseField, "for a compressed 8bit vector",
 InstallMethod( NewVector, "for Is8BitVectorRep, GF(q), and a list",
   [ Is8BitVectorRep, IsField and IsFinite, IsList ],
   function( filter, f, l )
+    if not Size(f) in [3..256] then
+        Error("Is8BitVectorRep only supports base fields with 3 to 256 elements");
+    fi;
     return CopyToVectorRep(l,Size(f));
   end );
 
 InstallMethod( NewZeroVector, "for Is8BitVectorRep, GF(q), and an int",
   [ Is8BitVectorRep, IsField and IsFinite, IsInt ],
   function( filter, f, i )
     local v;
+    if not Size(f) in [3..256] then
+        Error("Is8BitVectorRep only supports base fields with 3 to 256 elements");
+    fi;
     v := ListWithIdenticalEntries(i,Zero(f));
-    ConvertToVectorRep(v,Size(f));
+    CONV_VEC8BIT(v,Size(f));
     return v;
   end );
 
-InstallMethod( ZeroMatrix, "for a compressed 8bit matrix",
-  [IsInt, IsInt, Is8BitMatrixRep],
-  function( rows, cols, m )
-    local l,i;
-    l := [];
-    for i in [1..rows] do
-        Add(l,ZeroVector(cols,m[1]));
-    od;
-    ConvertToMatrixRep(l);
-    return l;
-  end );
-
 InstallMethod( NewMatrix, "for Is8BitMatrixRep, GF(q), an int, and a list",
   [ Is8BitMatrixRep, IsField and IsFinite, IsInt, IsList ],
   function( filter, f, rl, l )
     local m;
+    if not Size(f) in [3..256] then
+        Error("Is8BitMatrixRep only supports base fields with 3 to 256 elements");
+    fi;
     m := List(l,ShallowCopy);
     ConvertToMatrixRep(m,Size(f));
     return m;
@@ -1177,39 +1174,31 @@ InstallMethod( NewZeroMatrix, "for Is8BitMatrixRep, GF(q), and two ints",
   [ Is8BitMatrixRep, IsField and IsFinite, IsInt, IsInt ],
   function( filter, f, rows, cols )
     local m,i;
+    if not Size(f) in [3..256] then
+        Error("Is8BitMatrixRep only supports base fields with 3 to 256 elements");
+    fi;
+    if rows = 0 then
+        Error("Is8BitMatrixRep with zero rows not yet supported");
+    fi;
     m := 0*[1..rows];
     m[1] := NewZeroVector(Is8BitVectorRep,f,cols);
     for i in [2..rows] do
         m[i] := ShallowCopy(m[1]);
     od;
-    ConvertToMatrixRep(m,Size(f));
+    ConvertToMatrixRepNC(m,Size(f));
     return m;
   end );
 
-InstallMethod( IdentityMatrix, "for a compressed 8bit matrix",
-  [IsInt, Is8BitMatrixRep],
-  function(rows,m)
-    local f,n;
-    f := BaseField(m);
-    n := IdentityMat(rows,f);
-    ConvertToMatrixRep(n,Size(f));
-    return n;
-  end );
-
 InstallMethod( NewIdentityMatrix, "for Is8BitMatrixRep, GF(q), and an int",
   [ Is8BitMatrixRep, IsField and IsFinite, IsInt ],
-  function( filter, f, rows )
-    local m,i,o;
-    m := 0*[1..rows];
-    o := One(f);
-    m[1] := NewZeroVector(Is8BitVectorRep,f,rows);
-    for i in [2..rows] do
-        m[i] := ShallowCopy(m[1]);
-        m[i][i] := o;
+  function( filter, basedomain, dim )
+    local mat, one, i;
+    mat := NewZeroMatrix(filter, basedomain, dim, dim);
+    one := One(basedomain);
+    for i in [1..dim] do
+        mat[i,i] := one;
     od;
-    m[1][1] := o;
-    ConvertToMatrixRep(m,Size(f));
-    return m;
+    return mat;
   end );
 
 InstallMethod( ChangedBaseDomain, "for an 8bit vector and a finite field",

hpcgap/lib/vecmat.gi

@@ -2455,41 +2455,22 @@ InstallMethod( BaseField, "for a compressed gf2 vector",
 InstallMethod( NewVector, "for IsGF2VectorRep, GF(2), and a list",
   [ IsGF2VectorRep, IsField and IsFinite, IsList ],
   function( filter, f, l )
+    if Size(f) <> 2 then Error("IsGF2VectorRep only supported over GF(2)"); fi;
     return CopyToVectorRep(l,2);
   end );
 
-InstallMethod( ZeroMatrix, "for a compressed gf2 matrix",
-  [IsInt, IsInt, IsGF2MatrixRep],
-  function( rows, cols, m )
-    local l,i,x;
-    l := [];
-    x := m[1];
-    for i in [1..rows] do
-        Add(l,ZeroVector(cols,x));
-    od;
-    ConvertToMatrixRepNC(l,2);
-    return l;
-  end );
-
 InstallMethod( NewZeroVector, "for IsGF2VectorRep, GF(2), and an int",
   [ IsGF2VectorRep, IsField and IsFinite, IsInt ],
   function( filter, f, i )
+    if Size(f) <> 2 then Error("IsGF2VectorRep only supported over GF(2)"); fi;
     return ZERO_GF2VEC_2(i);
   end );
 
-InstallMethod( IdentityMatrix, "for a compressed gf2 matrix",
-  [IsInt, IsGF2MatrixRep],
-  function(rows,m)
-    local n;
-    n := IdentityMat(rows,GF(2));
-    ConvertToMatrixRepNC(n,2);
-    return n;
-  end );
-
 InstallMethod( NewMatrix, "for IsGF2MatrixRep, GF(2), an int, and a list",
   [ IsGF2MatrixRep, IsField and IsFinite, IsInt, IsList ],
   function( filter, f, rl, l )
     local m;
+    if Size(f) <> 2 then Error("IsGF2MatrixRep only supported over GF(2)"); fi;
     m := List(l,ShallowCopy);
     ConvertToMatrixRep(m,2);
     return m;
@@ -2499,29 +2480,28 @@ InstallMethod( NewZeroMatrix, "for IsGF2MatrixRep, GF(2), and two ints",
   [ IsGF2MatrixRep, IsField and IsFinite, IsInt, IsInt ],
   function( filter, f, rows, cols )
     local m,i;
-    m := 0*[1..rows];
-    m[1] := NewZeroVector(IsGF2VectorRep,f,cols);
-    for i in [2..rows] do
-        m[i] := ShallowCopy(m[1]);
+    if Size(f) <> 2 then Error("IsGF2MatrixRep only supported over GF(2)"); fi;
+    if rows = 0 then
+        Error("IsGF2MatrixRep with zero rows not yet supported");
+    fi;
+    m := EmptyPlist(rows);
+    for i in [1..rows] do
+        m[i] := ZERO_GF2VEC_2(cols);
     od;
-    ConvertToMatrixRep(m,2);
+    ConvertToMatrixRepNC(m,2);
     return m;
   end );
 
 InstallMethod( NewIdentityMatrix, "for IsGF2MatrixRep, GF(2), and an int",
   [ IsGF2MatrixRep, IsField and IsFinite, IsInt ],
-  function( filter, f, rows )
-    local m,i,o;
-    m := 0*[1..rows];
-    o := Z(2);
-    m[1] := NewZeroVector(IsGF2VectorRep,f,rows);
-    for i in [2..rows] do
-        m[i] := ShallowCopy(m[1]);
-        m[i,i] := o;
+  function( filter, basedomain, dim )
+    local mat, one, i;
+    mat := NewZeroMatrix(filter, basedomain, dim, dim);
+    one := One(basedomain);
+    for i in [1..dim] do
+        mat[i,i] := one;
     od;
-    m[1,1] := o;
-    ConvertToMatrixRep(m,2);
-    return m;
+    return mat;
   end );
 
 InstallMethod( ChangedBaseDomain, "for a gf2 vector and a finite field",

lib/mat8bit.gi

@@ -150,7 +150,7 @@ InstallMethod( ViewObj, "for a compressed MatFFE",
     local r,c;
     r := m![1];
     c := LEN_VEC8BIT(m![2]);
-    if r*c > 25 then
+    if r*c > 25 or r = 0 or c = 0 then
         Print("< ");
         if not IsMutable(m) then
             Print("im");

lib/matobj.gi

@@ -222,13 +222,22 @@ end);
 
 #############################################################################
 ##
-#M  NewZeroVector( <filt>, <R>, <n> )
+#M  ZeroVector( <filt>, <R>, <len> )
+#M  ZeroVector( <R>, <len> )
 ##
-InstallMethod( NewZeroVector,
-    "for filter, semiring, integer",
-    [ IsVectorObj, IsSemiring, IsInt ],
-    { filt, R, n } -> NewVector( filt, R, ListWithIdenticalEntries( n,
-                                              Zero( R ) ) ) );
+InstallMethod( ZeroVector,
+  [ IsOperation, IsSemiring, IsInt ],
+  function( rep, basedomain, len )
+    if len < 0 then Error("ZeroVector: length must be non-negative"); fi;
+    return NewZeroVector( rep, basedomain, len );
+  end );
+
+InstallMethod( ZeroVector,
+  [ IsSemiring, IsInt ],
+  function( basedomain, len )
+    if len < 0 then Error("ZeroVector: length must be non-negative"); fi;
+    return NewZeroVector( DefaultVectorRepForBaseDomain(basedomain), basedomain, len );
+  end );
 
 
 #############################################################################
@@ -380,20 +389,22 @@ InstallMethod( Matrix,
 InstallMethod( ZeroMatrix,
   [IsInt, IsInt, IsMatrixObj],
   function( rows, cols, example )
-    return ZeroMatrix( ConstructingFilter(example), BaseDomain(example), rows, cols );
+    if rows < 0 or cols < 0 then Error("ZeroMatrix: the number of rows and cols must be non-negative"); fi;
+    return NewZeroMatrix( ConstructingFilter(example), BaseDomain(example), rows, cols );
   end );
 
 InstallMethod( ZeroMatrix,
   [IsSemiring, IsInt, IsInt],
   function( basedomain, rows, cols )
-    return ZeroMatrix( DefaultMatrixRepForBaseDomain(basedomain), basedomain, rows, cols );
+    if rows < 0 or cols < 0 then Error("ZeroMatrix: the number of rows and cols must be non-negative"); fi;
+    return NewZeroMatrix( DefaultMatrixRepForBaseDomain(basedomain), basedomain, rows, cols );
   end );
 
 InstallMethod( ZeroMatrix,
   [IsOperation, IsSemiring, IsInt, IsInt],
   function( rep, basedomain, rows, cols )
-    # TODO: urge matrixobj implementors to overload this
-    return NewMatrix( rep, basedomain, cols, ListWithIdenticalEntries( rows * cols, Zero(basedomain) ) );
+    if rows < 0 or cols < 0 then Error("ZeroMatrix: the number of rows and cols must be non-negative"); fi;
+    return NewZeroMatrix( rep, basedomain, rows, cols );
   end );
 
 #
@@ -402,22 +413,27 @@ InstallMethod( ZeroMatrix,
 InstallMethod( IdentityMatrix,
   [IsInt, IsMatrixObj],
   function( dim, example )
-    return IdentityMatrix( ConstructingFilter(example), BaseDomain(example), dim );
+    if dim < 0 then Error("IdentityMatrix: the dimension must be non-negative"); fi;
+    return NewIdentityMatrix( ConstructingFilter(example), BaseDomain(example), dim );
   end );
 
 InstallMethod( IdentityMatrix,
   [IsSemiring, IsInt],
   function( basedomain, dim )
-    return IdentityMatrix( DefaultMatrixRepForBaseDomain(basedomain), basedomain, dim );
+    if dim < 0 then Error("IdentityMatrix: the dimension must be non-negative"); fi;
+    return NewIdentityMatrix( DefaultMatrixRepForBaseDomain(basedomain), basedomain, dim );
   end );
 
 InstallMethod( IdentityMatrix,
   [IsOperation, IsSemiring, IsInt],
   function( rep, basedomain, dim )
-    # TODO: avoid using IdentityMat eventually
-    return NewMatrix( rep, basedomain, dim, IdentityMat( dim, basedomain ) );
+    if dim < 0 then Error("IdentityMatrix: the dimension must be non-negative"); fi;
+    return NewIdentityMatrix( rep, basedomain, dim );
   end );
 
+#
+#
+#
 InstallMethod( CompanionMatrix,
     "for a polynomial and a matrix",
   [ IsUnivariatePolynomial, IsMatrixObj ],

lib/matobj2.gd

@@ -629,30 +629,42 @@ DeclareOperation( "ScalarProduct", [ IsVectorObj, IsVectorObj ] );
 
 #############################################################################
 ##
+#O  ZeroVector( <filt>, <R>, <len> )
+#O  ZeroVector( <R>, <len> )
 #O  ZeroVector( <len>, <v> )
 #O  ZeroVector( <len>, <M> )
 ##
 ##  <#GAPDoc Label="VectorObj_ZeroVector">
 ##  <ManSection>
 ##  <Heading>ZeroVector</Heading>
-##  <Oper Name="ZeroVector" Arg="l,v" Label="for length and vector object"/>
-##  <Oper Name="ZeroVector" Arg="l,M" Label="for length and matrix object"/>
+##  <Oper Name="ZeroVector" Arg="filt,R,len" Label="for filter, base domain and length"/>
+##  <Oper Name="ZeroVector" Arg="R,len" Label="for base domain and length"/>
+##  <Oper Name="ZeroVector" Arg="len,v" Label="for length and vector object"/>
+##  <Oper Name="ZeroVector" Arg="len,M" Label="for length and matrix object"/>
 ##
 ##  <Returns>a vector object</Returns>
 ##  <Description>
-##  For a vector object <A>v</A> and a nonnegative integer <A>l</A>,
-##  this operation returns a new mutable vector object of length <A>l</A>
+##  For a filter <A>filt</A>, a semiring <A>R</A> and a nonnegative integer <A>len</A>,
+##  this operation returns a new mutable vector object of length <A>len</A> over <A>R</A>
+##  in the representation <A>filt</A> containing only zeros.
+##  <P/>
+##  If only <A>R</A> and <A>len</A> are given, then GAP guesses a suitable representation.
+##  <P/>
+##  For a vector object <A>v</A> and a nonnegative integer <A>len</A>,
+##  this operation returns a new mutable vector object of length <A>len</A>
 ##  in the same representation as <A>v</A> containing only zeros.
 ##  <P/>
-##  For a matrix object <A>M</A> and a nonnegative integer <A>l</A>,
+##  For a matrix object <A>M</A> and a nonnegative integer <A>len</A>,
 ##  this operation returns a new mutable zero vector object of length
-##  <A>l</A> in the representation given by the
+##  <A>len</A> in the representation given by the
 ##  <Ref Attr="CompatibleVectorFilter" Label="for a matrix object"/> value
 ##  of <A>M</A>, provided that such a representation exists.
 ##  </Description>
 ##  </ManSection>
 ##  <#/GAPDoc>
 ##
+DeclareOperation( "ZeroVector", [ IsOperation, IsSemiring, IsInt ] );
+DeclareOperation( "ZeroVector", [ IsSemiring, IsInt ] );
 DeclareOperation( "ZeroVector", [ IsInt, IsVectorObj ] );
 DeclareOperation( "ZeroVector", [ IsInt, IsMatrixObj ] );
 
@@ -1898,9 +1910,11 @@ DeclareOperation( "[]:=", [ IsMatrixObj, IsPosInt, IsPosInt, IsObject ] );
 ##    And what about 'NullMat'/'ZeroMatrix' and
 ##    'IdentityMat'/'IdentityMatrix'?)
 ##
+##  - Are the argument names sensible and consistent? For example we sometimes
+##    write "rep" and sometimes "filt" for the same thing.
+##    And we sometimes write "rows, colss", other times "ncols", or "m,n", or...
+##
 ##  - Are the operations for vector and matrix objects consistent?
-##    For example, there are variants 'ZeroMatrix( [filt, ]R, m, n )'
-##    but no variants 'ZeroVector( [filt, ]R, l )'.
 ##
 ##  - How is 'WeightOfVector' related to 'WeightVecFFE', 'DistanceVecFFE'?
 ##    Should just 'WeightOfVector' be documented?

lib/matobjplist.gi

@@ -118,19 +118,14 @@ InstallMethod( NewZeroMatrix,
 InstallMethod( NewIdentityMatrix,
   "for IsPlistMatrixRep, a ring, and an int",
   [ IsPlistMatrixRep, IsRing, IsInt ],
-  function( filter, basedomain, rows )
-    local filterVectors, m, e, i;
-    filterVectors := IsPlistVectorRep;
-    m := 0*[1..rows];
-    e := NewVector(filterVectors, basedomain, []);
-    for i in [1..rows] do
-        m[i] := ZeroVector( rows, e );
-        m[i][i] := One(basedomain);
+  function( filter, basedomain, dim )
+    local mat, one, i;
+    mat := NewZeroMatrix(filter, basedomain, dim, dim);
+    one := One(basedomain);
+    for i in [1..dim] do
+        mat[i,i] := one;
     od;
-    m := [basedomain,e,rows,m];
-    Objectify( NewType(CollectionsFamily(FamilyObj(basedomain)),
-                       filter and IsMutable), m );
-    return m;
+    return mat;
   end );
 
 

lib/matrix.gi

@@ -1258,6 +1258,7 @@ end );
 InstallOtherMethod( BaseDomain,
     "generic method for a row vector",
     [ IsRowVector ],
+    -SUM_FLAGS,
     DefaultRing );
 
 InstallOtherMethod( OneOfBaseDomain,