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:

Status badges

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 pbruin

This is because the function sage.interfaces.gap.gfq_gap_to_sage, which converts a GAP element into an element of a Sage finite field F, constructs an intermediate field which does not necessarily have a canonical embedding into F. I am now testing a patch which avoids the intermediate field.

comment:2 Changed 6 years ago by pbruin

  • Authors set to Peter Bruin
  • 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 Sage FiniteField(p), as opposed to IntegerModRing(p).

comment:3 Changed 6 years ago by pbruin

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 vdelecroix

  • 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 field
  • DegreeFFE(z): degree of the smallest base field containing the element z
  • 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 vbraun

  • Branch changed from u/pbruin/18048-gap_finite_field_conversion to 61b6cb93796bc96ac72ce5eab6ecc46772335538
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.