Add error checks to ShortestVectors, cleanup (#6273)

Author: fingolfin

lib/zlattice.gi

@@ -1241,21 +1241,24 @@ end );
 ##
 #F  ShortestVectors( <mat>, <bound> [, \"positive\" ] )
 ##
-InstallGlobalFunction( ShortestVectors, function( arg )
+InstallGlobalFunction( ShortestVectors, function( a, m, arg... )
     local
     # variables
-          n,  checkpositiv, a, llg, nullv, m, c, con, b, v,
+          n, positiveOnly, llg, zeroCoeffs, c, continueSearch, b, v, i, j,
     # procedures
-          srt, vschr;
+          search, emitVector;
 
-    # search for shortest vectors
-    srt := function( d, dam )
-    local i, j, x, k, k1, q;
+    # Enumerate coefficient vectors recursively in the LLL-reduced basis,
+    # starting near the nearest integer to keep the search tree small.
+    search := function( d, norm )
+    local i, j, x, k, nextnorm, q;
     if d = 0 then
-       if v = nullv then
-          con := false;
+       # Once the zero coefficient vector is reached, the remaining branch
+       # would only enumerate the already covered opposite vectors.
+       if v = zeroCoeffs then
+          continueSearch := false;
        else
-          vschr( dam );
+          emitVector( norm );
        fi;
     else
        x := 0;
@@ -1267,27 +1270,27 @@ InstallGlobalFunction( ShortestVectors, function( arg )
           i := i - SignInt( x );
        fi;
        k := i + x;
-       q := ( m + 1/1000 - dam ) / llg.B[d];
+       q := ( m + 1/1000 - norm ) / llg.B[d];
        if k * k < q then
           repeat
              i := i + 1;
              k := k + 1;
           until k * k >= q and k > 0;
           i := i - 1;
           k := k - 1;
-          while k * k < q and con do
+          while k * k < q and continueSearch do
              v[d] := i;
-             k1 := llg.B[d] * k * k + dam;
-             srt( d-1, k1 );
+             nextnorm := llg.B[d] * k * k + norm;
+             search( d-1, nextnorm );
              i := i - 1;
              k := k - 1;
           od;
        fi;
     fi;
     end;
 
-    # output of vector
-    vschr := function( dam )
+    # Convert coefficients back to the original basis before storing them.
+    emitVector := function( norm )
     local newvec, i, j, w, haspos, hasneg;
     newvec := [];
     haspos := false;
@@ -1304,55 +1307,55 @@ InstallGlobalFunction( ShortestVectors, function( arg )
        fi;
        newvec[i] := w;
     od;
-    if checkpositiv then
+    if positiveOnly then
        if haspos and hasneg then
           return;
        elif hasneg then
           newvec := -newvec;
        fi;
     fi;
     Add(c.vectors, newvec);
-    Add(c.norms, dam);
+    Add(c.norms, norm);
     end;
 
     # main program
     # check input
-    if    not IsBound( arg[1] )
-       or not IsList( arg[1] ) or not IsList( arg[1][1] ) then
-       Error ( "first argument must be Gram matrix\n",
+    if not IsMatrixOrMatrixObj( a ) or NrRows( a ) <> NrCols( a ) then
+       Error ( "first argument must be a square Gram matrix\n",
           "usage: ShortestVectors( <mat>, <integer> [,<\"positive\">] )" );
-    elif not IsBound( arg[2] ) or not IsInt( arg[2] ) then
-       Error ( "second argument must be integer\n",
+    elif not IsInt( m ) or m < 0 then
+       Error ( "second argument must be a nonnegative integer\n",
           "usage: ShortestVectors( <mat>, <integer> [,<\"positive\">] )");
-    elif IsBound( arg[3] ) then
-       if IsString( arg[3] ) then
-          if arg[3] = "positive" then
-             checkpositiv := true;
-          else
-             checkpositiv := false;
-          fi;
+    elif IsBound( arg[1] ) then
+       if arg[1] = "positive" then
+          positiveOnly := true;
        else
-          Error ( "third argument must be string\n",
+          Error ( "third argument, if given, must be \"positive\"\n",
           "usage: ShortestVectors( <mat>, <integer> [,<\"positive\">] )");
        fi;
     else
-       checkpositiv := false;
+       positiveOnly := false;
     fi;
 
-    a := arg[1];
-    m := arg[2];
-    n := Length( a );
-    b := List( a, ShallowCopy );
+    if not IsSymmetricMatrix(a) then
+        Error ( "first argument must be a symmetric Gram matrix\n",
+           "usage: ShortestVectors( <mat>, <integer> [,<\"positive\">] )" );
+    fi;
+    n := NrRows( a );
+    b := MutableCopyMatrix( a );
     c     := rec( vectors:= [], norms:= [] );
     v     := ListWithIdenticalEntries( n, 0 );
-    nullv := ListWithIdenticalEntries( n, 0 );
+    zeroCoeffs := ListWithIdenticalEntries( n, 0 );
 
     llg:= LLLReducedGramMat( b );
-#T here check that the matrix is really regular
-#T (empty relations component)
+    if Length( llg.relations ) <> 0 then
+       Error ( "first argument must be a regular Gram matrix\n",
+          "usage: ShortestVectors( <mat>, <integer> [,<\"positive\">] )" );
+    fi;
 
-    con := true;
-    srt( n, 0 );
+    # The small offset avoids missing vectors on the exact norm boundary.
+    continueSearch := true;
+    search( n, 0 );
 
     Info( InfoZLattice, 2,
           "ShortestVectors: ", Length( c.vectors ), " vectors found" );
@@ -1703,14 +1706,14 @@ l := deca( l-1 );
        Error( "first argument must be symmetric Gram matrix\n",
               "usage : Orthog... ( < gram-matrix > \n",
               " [, <\"positive\"> ] [, < integer > ] )" );
-    elif Length( arg[1] ) <> Length( arg[1][1] ) then
+    elif NrRows( arg[1] ) <> NrCols( arg[1] ) then
        Error( "Gram matrix must be quadratic\n",
               "usage : Orthog... ( < gram-matrix >\n",
               " [, <\"positive\"> ] [, < integer > ] )" );
     fi;
     g := List( arg[1], ShallowCopy );
     checkdim := false;
-    chpo := "xxx";
+    chpo := fail;
     if IsBound( arg[2] ) then
        if IsString( arg[2] ) then
           chpo := arg[2];
@@ -1755,7 +1758,11 @@ l := deca( l-1 );
     invg  := Symmatinv( g );
     m     := invg.enuminator;
     invg  := invg.inverse;
-    x     := ShortestVectors( invg, m, chpo );
+    if chpo = "positive" then
+       x := ShortestVectors( invg, m, chpo );
+    else
+       x := ShortestVectors( invg, m );
+    fi;
     t     := Length(x.vectors);
     for i in [1..t] do
        x.vectors[i][n+1] := x.norms[i];

tst/testbugfix/2026-02-26-ShortestVectors-positive.tst

@@ -13,3 +13,15 @@ gap> Length( sv.vectors );
 120
 gap> ForAll( sv.vectors, x -> ForAll( x, y -> y >= 0 ) );
 true
+gap> ShortestVectors( [ [ 2 ] ], 2, "bogus" );
+Error, third argument, if given, must be "positive"
+usage: ShortestVectors( <mat>, <integer> [,<"positive">] )
+gap> ShortestVectors( [ [ 2 ] ], -1 );
+Error, second argument must be a nonnegative integer
+usage: ShortestVectors( <mat>, <integer> [,<"positive">] )
+gap> ShortestVectors( [ [ 1, 2 ], [ 0, 1 ] ], 1 );
+Error, first argument must be a symmetric Gram matrix
+usage: ShortestVectors( <mat>, <integer> [,<"positive">] )
+gap> ShortestVectors( [ [ 0 ] ], 1 );
+Error, first argument must be a regular Gram matrix
+usage: ShortestVectors( <mat>, <integer> [,<"positive">] )