Opened 10 years ago

Closed 9 years ago

#8332 closed enhancement (fixed)

Changes FiniteField_givaro to Python

Reported by: roed Owned by: davidloeffler
Priority: major Milestone: sage-4.4
Component: algebra Keywords:
Cc: Merged in: sage-4.4.alpha0
Authors: David Roe Reviewers: David Loeffler, John Cremona
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by cremona)

Residue fields and others would like to be able to multiply inherit from finite field parents. This is the first of the two switches needed to enable that (the other being sage.rings.finite_rings.element_ntl_gf2e.FiniteField_ntl_gf2e).

This depends on #8218.

Attachments (2)

trac_8332_givaro_python.patch (86.2 KB) - added by roed 10 years ago.
trac_8332_fix.patch (688 bytes) - added by davidloeffler 9 years ago.
apply over previous patch

Download all attachments as: .zip

Change History (10)

Changed 10 years ago by roed

comment:1 Changed 10 years ago by roed

  • Status changed from new to needs_review

comment:2 Changed 10 years ago by roed

Part of a series:

8218 -> 8332 -> 7880 -> 7883 -> 8333 -> 8334 -> 8335

I tried to make each of these mostly self contained, with doctests passing after every ticket, but I didn't entirely succeed. If you're reviewing one of these tickets, applying later tickets will hopefully fix doctest failures that you're seeing.

comment:3 Changed 10 years ago by davidloeffler

This applies fine to 4.3.4.rc0 (on top of 8218), and all doctests pass on 64-bit Linux except for a tiny failure in sage/structure/parent.pyx. This is just because some random code has used GF(9,'a') as an example of a Cython object, so it's trivial to fix. I am still waiting for Sage to build on T2, and once that happens I will test this there as well, but if that passes I think this is fine to go in.

comment:4 Changed 10 years ago by davidloeffler

(BTW, the aforementioned failure doesn't seem to be dealt with by any of the other tickets in the series -- none of them change that line of code.)

Changed 9 years ago by davidloeffler

apply over previous patch

comment:5 Changed 9 years ago by davidloeffler

  • Owner changed from AlexGhitza to davidloeffler

Here is a tiny patch to fix that failure. With this patch installed, all tests (including long) pass on selmer.warwick.ac.uk (Ubuntu x86_64), except an unrelated existing problem in sage/schemes/elliptic_curves/heegner.py; and a selection of tests in sage/rings/finite_rings pass on t2.math.washington.edu (Solaris) as well.

comment:6 follow-up: Changed 9 years ago by cremona

  • Description modified (diff)
  • Reviewers set to David Loeffler, John Cremona
  • Status changed from needs_review to positive_review

I applied both patches on top of a 4.3.5 build on 32-bit ubuntu, after previously applying the relevant bundle & patches from #8128.

With the first patch I tested all and found only the one failure mentioned aboue in sage/structures/parent.pyx. With the second patch this now passes.

Positive review! Now on to #7880...

comment:7 in reply to: ↑ 6 Changed 9 years ago by cremona

Replying to cremona:

I applied both patches on top of a 4.3.5 build on 32-bit ubuntu, after previously applying the relevant bundle & patches from #8128.

With the first patch I tested all and found only the one failure mentioned aboue in sage/structures/parent.pyx. With the second patch this now passes.

Positive review! Now on to #7880...

Sorry that should read #8218.

comment:8 Changed 9 years ago by jhpalmieri

  • Merged in set to sage-4.4.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

Merged into 4.4.alpha0:

  • trac_8332_givaro_python.patch
  • trac_8332_fix.patch
Note: See TracTickets for help on using tickets.