sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.3k stars 449 forks source link

Add a finite field implementation using FLINT's fq and fq_nmod modules #16664

Open jpflori opened 10 years ago

jpflori commented 10 years ago

Implement finite fields using flint fq and fq_nmod types.

Depends on #19646

CC: @defeo @pjbruin @sagetrac-erousseau

Component: finite rings

Keywords: flint finite field

Author: Jean-Pierre Flori

Branch/Commit: u/jpflori/flint_fq_nmod @ 9587ba6

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

jdemeyer commented 9 years ago
comment:64

Replying to @jpflori:

I only add new files needed by this ticket, I'm not sure it would make sense to add these files in another ticket.

But you were talking about adding # distutils: libraries = flint, weren't you? I would suggest to do this on a new ticket. So, when you're changing libs/flint anyway, you might as well add the new files too.

jpflori commented 9 years ago
comment:65

Replying to @jdemeyer:

Replying to @jpflori:

I only add new files needed by this ticket, I'm not sure it would make sense to add these files in another ticket.

But you were talking about adding # distutils: libraries = flint, weren't you? I would suggest to do this on a new ticket. So, when you're changing libs/flint anyway, you might as well add the new files too.

Oh yes, the distutils part should definitely be done elsewhere.

videlec commented 9 years ago
comment:66

Hello,

In _element_constructor_ the following case should not happen

if isinstance(x, self.element_class) and x.parent() is self:

as if parent(x) is self then this is catched by the __call__ of Parent. See line 1080-1083 of parent.pyx

        cdef R = parent_c(x) 
        cdef bint no_extra_args = len(args) == 0 and len(kwds) == 0 
        if R is self and no_extra_args: 
            return x

Instead of making __nonzero__ relies on is_unit I would rather implement __nonzero__ and call it in bot is_unit and is_zero. BTW for the latter this is what is in Element.is_zero by default, so you can even remove it. The reason why is that there is a special slot for __nonzero__ in Python objects. So the fastest (Python) way to do things should be

if not my_element:
    XYZ

and not

if my_element.is_zero():
    XYZ

What are the status of the flint patches? Are there incorporated in flint-2.5? If that is so, it would make sense to first include it into Sage.

Vincent

jpflori commented 9 years ago
comment:67

Replying to @videlec:

Hello,

In _element_constructor_ the following case should not happen

if isinstance(x, self.element_class) and x.parent() is self:

as if parent(x) is self then this is catched by the __call__ of Parent. See line 1080-1083 of parent.pyx

        cdef R = parent_c(x) 
        cdef bint no_extra_args = len(args) == 0 and len(kwds) == 0 
        if R is self and no_extra_args: 
            return x

Instead of making __nonzero__ relies on is_unit I would rather implement __nonzero__ and call it in bot is_unit and is_zero. BTW for the latter this is what is in Element.is_zero by default, so you can even remove it. The reason why is that there is a special slot for __nonzero__ in Python objects. So the fastest (Python) way to do things should be

if not my_element:
    XYZ

and not

if my_element.is_zero():
    XYZ

Thanks, I'll have a look at both of these. Not that the code is mainly cpoied/pasted from the PARI wrapper by Peter Bruin. I'll also check the PARI wrapper. Note the PARI wrapper is Python rather than Cython (as we have already have a cython "proxy" for PARI elements).

What are the status of the flint patches? Are there incorporated in flint-2.5? If that is so, it would make sense to first include it into Sage.

I guess so. Integrating flint 2.5 first is a good idea anyway. It will also allow to use the lacking in flint 2.4 fq_div function.

Vincent

videlec commented 9 years ago
comment:68

Replying to @jpflori:

Replying to @videlec:

What are the status of the flint patches? Are there incorporated in flint-2.5? If that is so, it would make sense to first include it into Sage.

I guess so. Integrating flint 2.5 first is a good idea anyway. It will also allow to use the lacking in flint 2.4 fq_div function.

This is #19009 (needs review).

videlec commented 9 years ago
comment:69

Conflicts with the flint upgrade (#19009).

jdemeyer commented 8 years ago

Changed dependencies from #15015, #17470 to #19646

jdemeyer commented 8 years ago
comment:71

Conflicts with 6.10.beta6. If you fix this conflict, I can easily merge #19646.

jpflori commented 7 years ago
comment:73

I guess the branch here is now completely broken. A more current implementation can be found at https://github.com/defeo/ffisom/