Skip to content

Commit

Permalink
kernel: use ffdata.h constants in finite field code
Browse files Browse the repository at this point in the history
This allows us in principle to support values of MAXSIZE_GF_INTERNAL other
than 2^16. But even if we never want to do that, it also removes some magic
constants from the kernel and arguably makes things more conceptual and
clearer.

Also adjust a few test cases, and made some error messages consistent
between library and kernel.
  • Loading branch information
fingolfin committed Apr 20, 2022
1 parent b9435f9 commit a5f054a
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 40 deletions.
8 changes: 8 additions & 0 deletions etc/ffgen.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ void emit_code(FILE * dest, int header)
fprintf(dest, "extern const UInt1 DegrFF[NUM_SHORT_FINITE_FIELDS+1];\n");
fprintf(dest, "extern const UInt4 CharFF[NUM_SHORT_FINITE_FIELDS+1];\n");
fprintf(dest, "\n");
if (num_ff < 65536)
fprintf(dest, "typedef UInt2 FF;\n");
else
fprintf(dest, "typedef UInt4 FF;\n");
if (MAX_FF <= 65536)
fprintf(dest, "typedef UInt2 FFV;\n");
else
fprintf(dest, "typedef UInt4 FFV;\n");
fprintf(dest, "\n");
fprintf(dest, "#endif // GAP_FFDATA_H\n");
}
Expand Down
4 changes: 2 additions & 2 deletions hpcgap/lib/ffeconway.gi
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ InstallOtherMethod(ZOp,
function(p,d)
local q;
if not IsPrimeInt(p) then
Error("Z: <p> must be a prime");
Error("Z: <p> must be a prime (not the integer ", p, ")");
fi;
q := p^d;
if q <= MAXSIZE_GF_INTERNAL or d =1 then
Expand All @@ -191,7 +191,7 @@ InstallMethod(ZOp,
d := LogInt(q,p);
Assert(1, q=p^d);
if not IsPrimeInt(p) then
Error("Z: <q> must be a positive prime power");
Error("Z: <q> must be a positive prime power (not the integer ", q, ")");
fi;
if d > 1 then
return FFECONWAY.ZNC(p,d);
Expand Down
4 changes: 2 additions & 2 deletions lib/ffeconway.gi
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ InstallOtherMethod(ZOp,
function(p,d)
local q;
if not IsPrimeInt(p) then
Error("Z: <p> must be a prime");
Error("Z: <p> must be a prime (not the integer ", p, ")");
fi;
q := p^d;
if q <= MAXSIZE_GF_INTERNAL or d =1 then
Expand All @@ -178,7 +178,7 @@ InstallMethod(ZOp,
d := LogInt(q,p);
Assert(1, q=p^d);
if not IsPrimeInt(p) then
Error("Z: <q> must be a positive prime power");
Error("Z: <q> must be a positive prime power (not the integer ", q, ")");
fi;
if d > 1 then
return FFECONWAY.ZNC(p,d);
Expand Down
19 changes: 10 additions & 9 deletions src/finfield.c
Original file line number Diff line number Diff line change
Expand Up @@ -1419,10 +1419,10 @@ static Obj FuncZ(Obj self, Obj q)
FF ff; /* the finite field */

/* check the argument */
if ( (IS_INTOBJ(q) && (INT_INTOBJ(q) > 65536)) ||
(TNUM_OBJ(q) == T_INTPOS))
return CALL_1ARGS(ZOp, q);
if ((IS_INTOBJ(q) && (INT_INTOBJ(q) > MAXSIZE_GF_INTERNAL)) ||
(TNUM_OBJ(q) == T_INTPOS))
return CALL_1ARGS(ZOp, q);

if ( !IS_INTOBJ(q) || INT_INTOBJ(q)<=1 ) {
RequireArgument(SELF_NAME, q, "must be a positive prime power");
}
Expand All @@ -1445,20 +1445,21 @@ static Obj FuncZ2(Obj self, Obj p, Obj d)
if (ARE_INTOBJS(p, d)) {
ip = INT_INTOBJ(p);
id = INT_INTOBJ(d);
if (ip > 1 && id > 0 && id <= 16 && ip < 65536) {
if (ip > 1 && id > 0 && id <= DEGREE_LARGEST_INTERNAL_FF &&
ip <= MAXSIZE_GF_INTERNAL) {
id1 = id;
q = ip;
while (--id1 > 0 && q <= 65536)
while (--id1 > 0 && q <= MAXSIZE_GF_INTERNAL)
q *= ip;
if (q <= 65536) {
if (q <= MAXSIZE_GF_INTERNAL) {
/* get the finite field */
ff = FiniteField(ip, id);
ff = FiniteFieldBySize(q);

if (ff == 0 || CHAR_FF(ff) != ip)
RequireArgument(SELF_NAME, p, "must be a prime");

/* make the root */
return NEW_FFE(ff, (ip == 2 && id == 1 ? 1 : 2));
return NEW_FFE(ff, (q == 2) ? 1 : 2);
}
}
}
Expand Down
48 changes: 25 additions & 23 deletions src/finfield.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
**
** Finite fields are an important domain in computational group theory
** because the classical matrix groups are defined over those finite fields.
** In GAP we support small finite fields with up to 65536 elements,
** larger fields can be realized as polynomial domains over smaller fields.
** The GAP kernel supports elements of finite fields up to some fixed size
** limit stored in MAXSIZE_GF_INTERNAL. To change this limit for 32 resp.
** 64 bit systems, edit `etc/ffgen.c`. Support for fields larger than this
** is implemented by the GAP library.
**
** Elements in small finite fields are represented as immediate objects.
**
Expand All @@ -24,10 +26,11 @@
** The least significant 3 bits of such an immediate object are always 010,
** flagging the object as an object of a small finite field.
**
** The next 13 bits represent the small finite field where the element lies.
** They are simply an index into a global table of small finite fields.
** The next group of FIELD_BITS_FFE bits represent the small finite field
** where the element lies. They are simply an index into a global table of
** small finite fields, which is constructed at build time.
**
** The most significant 16 bits represent the value of the element.
** The most significant VAL_BITS_FFE bits represent the value of the element.
**
** If the value is 0, then the element is the zero from the finite field.
** Otherwise the integer is the logarithm of this element with respect to a
Expand Down Expand Up @@ -69,10 +72,14 @@
**
** Small finite fields are represented by an index into a global table.
**
** Since there are only 6542 (prime) + 93 (nonprime) small finite fields,
** the index fits into a 'UInt2' (actually into 13 bits).
** Depending on the configuration it may be UInt2 or UInt4. The definition
** is in `ffdata.h` and is calculated by `etc/ffgen.c`
*/
typedef UInt2 FF;
GAP_STATIC_ASSERT(NUM_SHORT_FINITE_FIELDS <= (1<<(8*sizeof(FF))),
"NUM_SHORT_FINITE_FIELDS too large for type FF");

GAP_STATIC_ASSERT(FIELD_BITS_FFE + VAL_BITS_FFE + 3 <= 8*sizeof(Obj),
"not enough bits in type Obj to store internal FFEs");


/****************************************************************************
Expand Down Expand Up @@ -140,17 +147,11 @@ extern Obj SuccFF;
** Values of elements of small finite fields are represented by the
** logarithm of the element with respect to the root plus one.
**
** Since small finite fields contain at most 65536 elements, the value fits
** into a 'UInt2'.
**
** It may be possible to change this to 'UInt4' to allow small finite fields
** with more than 65536 elements. The macros and have been coded in
** such a way that they work without problems. The exception is 'POW_FFV'
** which will only work if the product of integers of type 'FFV' does not
** cause an overflow. And of course the successor table stored for a finite
** field will become quite large for fields with more than 65536 elements.
** Depending on the configuration, this type may be a UInt2 or UInt4.
** This type is actually defined in `ffdata.h` by `etc/ffgen.c`
*/
typedef UInt2 FFV;
GAP_STATIC_ASSERT(MAXSIZE_GF_INTERNAL <= (1<<(8*sizeof(FFV))),
"MAXSIZE_GF_INTERNAL too large for type FFV");

GAP_STATIC_ASSERT(sizeof(UInt) >= 2 * sizeof(FFV),
"Overflow possibility in POW_FFV");
Expand Down Expand Up @@ -288,8 +289,7 @@ EXPORT_INLINE FFV QUO_FFV(FFV a, FFV b, const FFV * f)
** in the range $0..order(f)-1$.
**
** Finally 'POW_FFV' may only be used if the product of two integers of the
** size of 'FFV' does not cause an overflow, i.e. only if 'FFV' is
** 'unsigned short'.
** size of 'FFV' does not cause an overflow.
**
** If the finite field element is 0 the power is also 0, otherwise we have
** $a^n ~ (z^{a-1})^n = z^{(a-1)*n} = z^{(a-1)*n % (o-1)} ~ (a-1)*n % (o-1)$
Expand Down Expand Up @@ -320,7 +320,7 @@ EXPORT_INLINE FFV POW_FFV(FFV a, UInt n, const FFV * f)
EXPORT_INLINE FF FLD_FFE(Obj ffe)
{
GAP_ASSERT(IS_FFE(ffe));
return (FF)((((UInt)(ffe)) & 0xFFFF) >> 3);
return (FF)((UInt)(ffe) >> 3) & ((1 << FIELD_BITS_FFE) - 1);
}


Expand All @@ -336,7 +336,8 @@ EXPORT_INLINE FF FLD_FFE(Obj ffe)
EXPORT_INLINE FFV VAL_FFE(Obj ffe)
{
GAP_ASSERT(IS_FFE(ffe));
return (FFV)(((UInt)(ffe)) >> 16);
return (FFV)((UInt)(ffe) >> (3 + FIELD_BITS_FFE)) &
((1 << VAL_BITS_FFE) - 1);
}


Expand All @@ -351,7 +352,8 @@ EXPORT_INLINE FFV VAL_FFE(Obj ffe)
EXPORT_INLINE Obj NEW_FFE(FF fld, FFV val)
{
GAP_ASSERT(val < SIZE_FF(fld));
return (Obj)(((UInt)(val) << 16) + ((UInt)(fld) << 3) + (UInt)0x02);
return (Obj)(((UInt)val << (3 + FIELD_BITS_FFE)) | ((UInt)fld << 3) |
(UInt)0x02);
}


Expand Down
10 changes: 6 additions & 4 deletions tst/testinstall/ffe.tst
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ gap> Z(-2);
Error, Z: <q> must be a positive prime power (not the integer -2)
gap> Z(6);
Error, Z: <q> must be a positive prime power (not the integer 6)
gap> Z(65537*65539);
Error, Z: <q> must be a positive prime power (not the integer 4295229443)

# variant with two arguments
gap> Z(0,1);
Expand Down Expand Up @@ -65,13 +67,13 @@ Error, Z: <p> must be a prime (not the integer 9)
gap> Z(9,2);
Error, Z: <p> must be a prime (not the integer 9)
gap> Z(2^16,1);
Error, Z: <p> must be a prime
Error, Z: <p> must be a prime (not the integer 65536)
gap> Z(2^16,2);
Error, Z: <p> must be a prime
Error, Z: <p> must be a prime (not the integer 65536)
gap> Z(2^17,1);
Error, Z: <p> must be a prime
Error, Z: <p> must be a prime (not the integer 131072)
gap> Z(2^17,2);
Error, Z: <p> must be a prime
Error, Z: <p> must be a prime (not the integer 131072)
# Invoking Z(p,d) with p not a prime used to crash gap, which we fixed.
# However, invocations like `Z(4,5)` still would erroneously trigger the
Expand Down

0 comments on commit a5f054a

Please sign in to comment.