Skip to content

Commit

Permalink
kernel: introduce FiniteFieldBySize
Browse files Browse the repository at this point in the history 
Also improve the error messages for `Z`
  • Loading branch information
@stevelinton @fingolfin
stevelinton authored and fingolfin committed Jan 7, 2019
1 parent 9441fd7 commit 823bd43
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 81 deletions.
89 changes: 37 additions & 52 deletions src/finfield.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,32 +112,29 @@ static FF LookupPrimePower(UInt q)

/****************************************************************************
**
*F FiniteField(<p>,<d>) . . . make the small finite field with <q> elements
*F FiniteField(<p>,<d>) . make the small finite field with <p>^<d> elements
*F FiniteFieldBySize(<q>) . . make the small finite field with <q> elements
**
** 'FiniteField' returns the small finite field with <p>^<d> elements.
** The work is done in the Lookup function above, and in FiniteFieldBySize
** where the successor tables are computed.
*/
FF FiniteField (
UInt p,
UInt d )

FF FiniteFieldBySize(UInt q)
{
FF ff; /* finite field, result */
Obj tmp; /* temporary bag */
Obj succBag; /* successor table bag */
FFV * succ; /* successor table */
FFV * indx; /* index table */
UInt q; /* size of finite field */
UInt p; /* characteristic of the field */
UInt poly; /* Conway polynomial of extension */
UInt i, l, f, n, e; /* loop variables */
Obj root; /* will be a primitive root mod p */

/* calculate size of field */
q = 1;
for (i = 1; i <= d; i++)
q *= p;

ff = LookupPrimePower(q);
if (CharFF[ff] != p)
if (!ff)
return 0;

#ifdef HPCGAP
/* Important correctness concern here:
*
Expand All @@ -162,6 +159,9 @@ FF FiniteField (
return ff;
#endif

// determine the characteristic of the field
p = CHAR_FF(ff);

/* allocate a bag for the successor table and one for a temporary */
tmp = NewBag(T_DATOBJ, sizeof(Obj) + q * sizeof(FFV));
SET_TYPE_DATOBJ(tmp, TYPE_KERNEL_OBJECT);
Expand All @@ -174,10 +174,11 @@ FF FiniteField (

/* if q is a prime find the smallest primitive root $e$, use $x - e$ */

if (d == 1) {
if (DEGR_FF(ff) == 1) {
if (p < 65537) {
/* for smaller primes we do this in the kernel for performance and
bootstrapping reasons */
bootstrapping reasons
TODO -- review the threshold */
for (e = 1, i = 1; i != p - 1; ++e) {
for (f = e, i = 1; f != 1; ++i)
f = (f * e) % p;
Expand Down Expand Up @@ -256,6 +257,21 @@ FF FiniteField (
return ff;
}

FF FiniteField(UInt p, UInt d)
{
UInt q, i;
FF ff;

q = 1;
for (i = 1; i <= d; i++)
q *= p;

ff = FiniteFieldBySize(q);
if (ff != 0 && CHAR_FF(ff) != p)
return 0;
return ff;
}


/****************************************************************************
**
Expand Down Expand Up @@ -1480,60 +1496,29 @@ Obj FuncINT_FFE_DEFAULT (
*/
static Obj ZOp;




Obj FuncZ (
Obj self,
Obj q )
{
FF ff; /* the finite field */
UInt p; /* characteristic */
UInt d; /* degree */
UInt r; /* temporary */

/* 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)<=1 ) {
q = ErrorReturnObj(
"Z: <q> must be a positive prime power (not a %s)",
(Int)TNAM_OBJ(q), 0L,
"you can replace <q> via 'return <q>;'" );
return FuncZ( self, q );
RequireArgument("Z", q, "must be a positive prime power");
}

/* compute the prime and check that <q> is a prime power */
if ( INT_INTOBJ(q) % 2 == 0 ) {
p = 2;
}
else {
p = 3;
while ( INT_INTOBJ(q) % p != 0 ) {
p += 2;
}
}
d = 1;
r = p;
while ( r < INT_INTOBJ(q) ) {
d = d + 1;
r = r * p;
}
if ( r != INT_INTOBJ(q) ) {
q = ErrorReturnObj(
"Z: <q> must be a positive prime power (not a %s)",
(Int)TNAM_OBJ(q), 0L,
"you can replace <q> via 'return <q>;'" );
return FuncZ( self, q );
}
ff = FiniteFieldBySize(INT_INTOBJ(q));

/* get the finite field */
ff = FiniteField( p, d );
if (!ff) {
RequireArgument("Z", q, "must be a positive prime power");
}

/* make the root */
return NEW_FFE( ff, (p == 2 && d == 1 ? 1 : 2) );
return NEW_FFE(ff, (q == INTOBJ_INT(2)) ? 1 : 2);
}

Obj FuncZ2 ( Obj self, Obj p, Obj d)
Expand All @@ -1554,7 +1539,7 @@ Obj FuncZ2 ( Obj self, Obj p, Obj d)
ff = FiniteField(ip, id);

if (ff == 0 || CHAR_FF(ff) != ip)
ErrorMayQuit("Z: <p> must be a prime", 0, 0);
RequireArgument("Z", p, "must be a prime");

/* make the root */
return NEW_FFE(ff, (ip == 2 && id == 1 ? 1 : 2));
Expand Down
9 changes: 5 additions & 4 deletions src/finfield.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,13 +348,14 @@ EXPORT_INLINE Obj NEW_FFE(FF fld, FFV val)

/****************************************************************************
**
*F FiniteField(<p>,<d>) . . . make the small finite field with <q> elements
*F FiniteField(<p>,<d>) . make the small finite field with <p>^<d> elements
*F FiniteFieldBySize(<q>) . . make the small finite field with <q> elements
**
** 'FiniteField' returns the small finite field with <p>^<d> elements.
** 'FiniteFieldBySize' returns the small finite field with <q> elements.
*/
extern FF FiniteField (
UInt p,
UInt d );
extern FF FiniteField(UInt p, UInt d);
extern FF FiniteFieldBySize(UInt q);


/****************************************************************************
Expand Down
5 changes: 0 additions & 5 deletions tst/testbugfix/2016-12-20-t00349.tst
View file

This file was deleted.

13 changes: 0 additions & 13 deletions tst/testbugfix/2017-08-07-FFE-bad-char.tst
View file

This file was deleted.

30 changes: 23 additions & 7 deletions tst/testinstall/ffe.tst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ gap> List([2,3,4,5,7,8,9,25,37^3], Z);

# input validation
gap> Z(fail);
Error, Z: <q> must be a positive prime power (not a boolean or fail)
Error, Z: <q> must be a positive prime power (not the value 'fail')
gap> Z(0);
Error, Z: <q> must be a positive prime power (not a integer)
Error, Z: <q> must be a positive prime power (not the integer 0)
gap> Z(1);
Error, Z: <q> must be a positive prime power (not a integer)
Error, Z: <q> must be a positive prime power (not the integer 1)
gap> Z(-2);
Error, Z: <q> must be a positive prime power (not a integer)
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 a integer)
Error, Z: <q> must be a positive prime power (not the integer 6)

# variant with two arguments
gap> Z(0,1);
Expand Down Expand Up @@ -58,10 +58,12 @@ gap> Z(bigPrime^2) = Z(bigPrime,2);
true
# verify some edge cases which previously were accepted (incorrectly)
gap> Z(6,3);
Error, Z: <p> must be a prime (not the integer 6)
gap> Z(9,1);
Error, Z: <p> must be a prime
Error, Z: <p> must be a prime (not the integer 9)
gap> Z(9,2);
Error, Z: <p> must be a prime
Error, Z: <p> must be a prime (not the integer 9)
gap> Z(2^16,1);
Error, Z: <p> must be a prime
gap> Z(2^16,2);
Expand All @@ -71,6 +73,20 @@ Error, Z: <p> must be a prime
gap> Z(2^17,2);
Error, Z: <p> must be a prime
# 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
# creation of a type object for fields of size p^d (in the example: 1024),
# with the non-prime value p set as characteristic. This could then corrupt
# subsequent computations.
gap> Z(4,5);
Error, Z: <p> must be a prime (not the integer 4)
gap> FieldByGenerators(GF(2), [ Z(1024) ]);
GF(2^10)
gap> Characteristic(Z(1024));
2
gap> Characteristic(FamilyObj(Z(1024)));
2
#
# Constructing finite fields and their subfields
#
Expand Down

0 comments on commit 823bd43

Please sign in to comment.