Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix finite field modulus handling #16983

Closed
jdemeyer opened this issue Sep 15, 2014 · 26 comments
Closed

Fix finite field modulus handling #16983

jdemeyer opened this issue Sep 15, 2014 · 26 comments

Comments

@jdemeyer
Copy link

For prime finite fields, we should support a custom modulus (internally in Sage, we don't need to use this modulus in the backend):

sage: x = polygen(GF(7))
sage: k = GF(7, modulus=x+5)
sage: k.modulus()
x + 6
sage: k.gen()
1

We should also make the modulus monic:

sage: x = polygen(GF(7))
sage: k.<a> = GF(7^2, modulus=2*x^2-3, impl="pari_ffelt")
sage: k.modulus()
2*x^2 + 4

(the Givaro backend does this implicitly)

The patch does the above, with lots of cleaning up.

Depends on #16934

CC: @pjbruin @jpflori

Component: finite rings

Author: Jeroen Demeyer

Branch/Commit: 0d9b5cc

Reviewer: Jean-Pierre Flori

Issue created by migration from https://trac.sagemath.org/ticket/16983

@jdemeyer jdemeyer added this to the sage-6.4 milestone Sep 15, 2014
@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

Dependencies: #16934

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

Author: Jeroen Demeyer

@jdemeyer
Copy link
Author

Branch: u/jdemeyer/ticket/16983

@jdemeyer
Copy link
Author

New commits:

8f805a2Trac 16934: always put finite field implementation in factory key
8923777Trac 16934: remove method FiniteFieldFactory.other_keys()
5e2681eTrac 16934: more cleaning up
ffb74faTrac 16934: add doctest
33c86efMerge remote-tracking branch 'origin/develop' into ticket/16934
bf36ec1Trac 16934: put back (temporary?) warning about modulus parameter
9cbdcdeTrac 16934: fix warning about ignored 'modulus' argument
eb5553eTrac #16983: no longer ignore modulus for prime finite fields

@jdemeyer
Copy link
Author

Commit: eb5553e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 16, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

b6a6c66Trivial changes to the FiniteField_pari_ffelt constructor

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 16, 2014

Changed commit from eb5553e to b6a6c66

@jdemeyer

This comment has been minimized.

@jpflori
Copy link

jpflori commented Oct 27, 2014

comment:9

I get a failing doctest in src/sage/rings/finite_rings/homset.py because some ordering changed.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

f7eccdcMerge remote-tracking branch 'origin/develop' into ticket/16983

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2014

Changed commit from b6a6c66 to f7eccdc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

dd16a75Fix order in doctest output

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2014

Changed commit from f7eccdc to dd16a75

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2014

Changed commit from dd16a75 to 5028e10

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

5028e10Fix infinite loop in unpickling

@jdemeyer
Copy link
Author

New commits:

5028e10Fix infinite loop in unpickling

@jpflori
Copy link

jpflori commented Oct 28, 2014

comment:15

There is something missing here:

+ def polynomial(self, name=None):
+ """
+ Return the irreducible characteristic polynomial of the
+ generator of this finite field, i.e., the polynomial `f(x)` so
+ elements of the finite field as elements modulo `f`.

@jpflori
Copy link

jpflori commented Oct 28, 2014

comment:16

Otherwise, this looks fine to me and all tests pass.
I'll give a final look a little bit later this afternoon.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

0d9b5ccClarify docs for polynomial()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2014

Changed commit from 5028e10 to 0d9b5cc

@jdemeyer
Copy link
Author

comment:18

Replying to @jpflori:

There is something missing here:

+ def polynomial(self, name=None):
+ """
+ Return the irreducible characteristic polynomial of the
+ generator of this finite field, i.e., the polynomial `f(x)` so
+ elements of the finite field as elements modulo `f`.

I just moved that sentence, in fact it dates back to

commit a4928b0cf450b765c3bde6e5328f0146603526a8
Author: tornaria <tornaria@math.utexas.edu>
Date:   Sat Feb 11 01:13:08 2006 +0000

    [project @ original sage-0.10.12]


New commits:

0d9b5ccClarify docs for polynomial()

@jpflori
Copy link

jpflori commented Oct 29, 2014

Reviewer: Jean-Pierre Flori

@jpflori
Copy link

jpflori commented Oct 29, 2014

comment:19

Ok, looks good to me.

@vbraun
Copy link
Member

vbraun commented Oct 29, 2014

Changed branch from u/jdemeyer/ticket/16983 to 0d9b5cc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants