Opened 6 years ago
Closed 6 years ago
#18048 closed defect (fixed)
Bug in GAP conversion of finite field elements
Reported by: | vdelecroix | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | sage-6.6 |
Component: | finite rings | Keywords: | |
Cc: | Merged in: | ||
Authors: | Peter Bruin | Reviewers: | Vincent Delecroix |
Report Upstream: | N/A | Work issues: | |
Branch: | 61b6cb9 (Commits, GitHub, GitLab) | Commit: | 61b6cb93796bc96ac72ce5eab6ecc46772335538 |
Dependencies: | Stopgaps: |
Description
As reported in this question on ask we have a problem with our finite fields and GAP
sage: K = GF(16,'a') sage: a = K.gen() sage: b = a**2 + a sage: K(b._gap_()) Traceback (most recent call last): ... TypeError: unable to coerce from a finite field other than the prime subfield
Strangely, it works with GF(8)
.
Change History (5)
comment:1 Changed 6 years ago by
comment:2 Changed 6 years ago by
- Branch set to u/pbruin/18048-gap_finite_field_conversion
- Commit set to 61b6cb93796bc96ac72ce5eab6ecc46772335538
- Status changed from new to needs_review
Notes:
- Before this patch, it was implicitly assumed that non-prime finite fields are defined by Conway polynomials, because this is how GAP represents finite fields. This is now an explicit requirement.
- Elements of a GAP prime field
Z(p)
are now converted into a SageFiniteField(p)
, as opposed toIntegerModRing(p)
.
comment:3 Changed 6 years ago by
In addition to the doctest, I tested the example given in the link in the ticket description 200 times and did not get any errors.
comment:4 Changed 6 years ago by
- Priority changed from major to critical
- Reviewers set to Vincent Delecroix
- Status changed from needs_review to positive_review
Hello,
That's wonderful that you did have a look! I put this to positive review as it fixes an important bug (and might hopefully be in sage-6.6).
On a related note, the code uses a lot of string parsing. This is ugly and not very safe. I think that we can get rid of most of it with
Characteristic(z)
: the characteristic of the fieldDegreeFFE(z)
: degree of the smallest base field containing the elementz
LogFFE(z,base)
: logarithm in finite field
I do not like the fact that we need to call LogFFE
to have access to the good power of the generator. If you think about a good solution for that I will open an other ticket.
Vincent
comment:5 Changed 6 years ago by
- Branch changed from u/pbruin/18048-gap_finite_field_conversion to 61b6cb93796bc96ac72ce5eab6ecc46772335538
- Resolution set to fixed
- Status changed from positive_review to closed
This is because the function
sage.interfaces.gap.gfq_gap_to_sage
, which converts a GAP element into an element of a Sage finite fieldF
, constructs an intermediate field which does not necessarily have a canonical embedding intoF
. I am now testing a patch which avoids the intermediate field.