Skip to content

Commit 09bcfdd

Browse files
committed
Use f.roots() instead of f.any_root() when working with extensions
1 parent 2569e61 commit 09bcfdd

File tree

1 file changed

+28
-15
lines changed

1 file changed

+28
-15
lines changed

src/sage/rings/polynomial/polynomial_element.pyx

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2612,19 +2612,28 @@ cdef class Polynomial(CommutativePolynomial):
26122612
# would work with the smallest possible extension.
26132613
# However, if we have some element of GF(p^k) and we try and coerce this to
26142614
# some element GF(p^(k*n)) this can fail, even though mathematically it
2615-
# should be fine. As a result, we simply coerce straight to the user supplied
2616-
# ring and find a root here.
2615+
# should be fine.
26172616
# TODO: Additionally, if the above was solved, it would be faster to extend the base
26182617
# ring with the irreducible factor however, if the base ring is an extension
26192618
# then the type of self.base_ring().extension(f) is a Univariate Quotient Polynomial Ring
2620-
# and not a finite field. So we do the slower option here to ensure the type is
2621-
# maintained.
2622-
if not f.degree().is_one():
2623-
f = f.change_ring(ring)
2624-
2625-
# Now we find the root of this irreducible polynomial over this extension
2626-
root = f.any_root()
2627-
return ring(root)
2619+
# and not a finite field.
2620+
2621+
# When f has degree one we simply return the roots
2622+
# TODO: should we write something fast for degree two using
2623+
# the quadratic formula?
2624+
if f.degree().is_one():
2625+
root = - f[0] / f[1]
2626+
return ring(root)
2627+
2628+
# TODO: The proper thing to do here would be to call
2629+
# return f.change_ring(ring).any_root()
2630+
# but as we cannot work in the minimal extension (see above) working
2631+
# in the extension for f.change_ring(ring).any_root() is almost always
2632+
# much much much slower than using the call for roots() which uses
2633+
# C library bindings for all finite fields.
2634+
# Until the coercion system for finite fields works better,
2635+
# this will be the most performant
2636+
return f.roots(ring, multiplicities=False)[0]
26282637

26292638
# The old version of `any_root()` allowed degree < 0 to indicate that the input polynomial
26302639
# had a distinct degree factorisation, we pass this to any_irreducible_factor as a bool and
@@ -2668,11 +2677,15 @@ cdef class Polynomial(CommutativePolynomial):
26682677
# over explicitly a FiniteField type.
26692678
ring = self.base_ring().extension(degree, names="a")
26702679

2671-
# Now we look for a linear root of this irreducible polynomial of degree `degree`
2672-
# over the user supplied ring or the extension we just computed. If the user sent
2673-
# a bad ring here of course there may be no root found.
2674-
f = f.change_ring(ring)
2675-
return f.any_root()
2680+
# TODO: The proper thing to do here would be to call
2681+
# return f.change_ring(ring).any_root()
2682+
# but as we cannot work in the minimal extension (see above) working
2683+
# in the extension for f.change_ring(ring).any_root() is almost always
2684+
# much much much slower than using the call for roots() which uses
2685+
# C library bindings for all finite fields.
2686+
# Until the coercion system for finite fields works better,
2687+
# this will be the most performant
2688+
return f.roots(ring, multiplicities=False)[0]
26762689

26772690
def __truediv__(left, right):
26782691
r"""

0 commit comments

Comments
 (0)