Skip to content

Commit

Permalink
gh-35358: Lighter construction of finite field elements from lists
Browse files Browse the repository at this point in the history
    
### 📚 Description

When doing intensive polynomial arithmetic with the NTL implementation
the constructor with lists is called a large number of times and may
spend a lot of time constructing the vector_space and FreeModuleElement
objects.

The very common call to vector_space(map=False) is optimized to be as
cheap as possible using the already cached object.

The common case of lists of length 0 and 1 is replaced by cheaper
shortcuts.

This improves performance when doing intensive polynomial computations
over finite field extensions.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x
]`. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.
    
URL: #35358
Reported by: Rémy Oudompheng
Reviewer(s): Travis Scrimshaw
  • Loading branch information
Release Manager committed Apr 4, 2023
2 parents d051998 + 63c3ab6 commit 65f4cd2
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
14 changes: 13 additions & 1 deletion src/sage/rings/finite_rings/element_pari_ffelt.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,19 @@ cdef class FiniteFieldElement_pari_ffelt(FinitePolyExtElement):
sage: k = FiniteField(3^11, 't', impl='pari_ffelt')
sage: k([ 0, 1/2 ])
2*t
sage: k([ 0, 1/2, 0, 0, 0, 0, 0, 0, 0, -1, 0 ])
2*t^9 + 2*t
sage: k([ k(0), k(1) ])
t
sage: k([ GF(3)(2), GF(3^5,'u')(1) ])
t + 2
sage: R.<x> = PolynomialRing(k)
sage: k([ x/x ])
1
sage: k([ R(-1), x/x ])
t + 2
sage: k([ R(-1), R(0), 0 ])
2
Check that zeros are created correctly (:trac:`11685`)::
Expand Down Expand Up @@ -496,7 +502,13 @@ cdef class FiniteFieldElement_pari_ffelt(FinitePolyExtElement):
self.construct_from(x.constant_coefficient())

elif isinstance(x, list):
if len(x) == self._parent.degree():
n = len(x)
if n == 0:
self.construct_from(None)
elif n == 1:
Fp = self._parent.base_ring()
self.construct_from(Fp(x[0]))
elif n == self._parent.degree():
self.construct_from(self._parent.vector_space(map=False)(x))
else:
Fp = self._parent.base_ring()
Expand Down
9 changes: 7 additions & 2 deletions src/sage/rings/finite_rings/finite_field_base.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1248,8 +1248,6 @@ cdef class FiniteField(Field):
sage: all(to_V(h(c) * e) == c * to_V(e) for e in E for c in F)
True
"""
from sage.modules.all import VectorSpace
from sage.categories.morphism import is_Morphism
if subfield is not None:
if base is not None:
raise ValueError
Expand All @@ -1259,6 +1257,13 @@ cdef class FiniteField(Field):
deprecation(28481, "The default value for map will be changing to True. To keep the current behavior, explicitly pass map=False.")
map = False

if base is None and self.__vector_space is not None and not map:
# A very common case: return as early as possible.
return self.__vector_space

from sage.modules.all import VectorSpace
from sage.categories.morphism import is_Morphism

if base is None:
base = self.prime_subfield()
s = self.degree()
Expand Down

0 comments on commit 65f4cd2

Please sign in to comment.