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

Total refactor of any_root() to solve issue in characteristic two and clean up the code #37170

Merged
merged 31 commits into from
Feb 13, 2024
Merged
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
852944b
Ugly patch for characteristic two in any_root
GiacomoPope Jan 26, 2024
356c881
Simply rewrite any_root, as it's easier than debugging the old code
GiacomoPope Jan 27, 2024
661efd8
handle deprecation properly
GiacomoPope Jan 27, 2024
a8dcf5a
Include any_irreducible_factor and refactor any_root to simply wrap this
GiacomoPope Jan 29, 2024
32066e2
Fix linting issues
GiacomoPope Jan 29, 2024
b810fcc
Small edits to fix review notes
GiacomoPope Jan 29, 2024
b240bcf
Add docstrings to helper functions
GiacomoPope Jan 29, 2024
f26534e
Remove the check for the zero polynomial
GiacomoPope Jan 29, 2024
66e1ae0
Too much whitespace
GiacomoPope Jan 29, 2024
24c2779
Fix refactoring bugs
GiacomoPope Jan 29, 2024
f7e157c
Try and make new function work like th eold one
GiacomoPope Jan 29, 2024
afa5f5f
white space
GiacomoPope Jan 29, 2024
aa5a290
Replace trac with issue throughout polynomial_element.pyx
GiacomoPope Jan 29, 2024
006fbae
Add doctests for new functions
GiacomoPope Jan 29, 2024
f0ba87e
every time, whitespace...
GiacomoPope Jan 29, 2024
c65548f
Ensure that the roots returned are of the expected type and add a not…
GiacomoPope Jan 30, 2024
e488817
Whitespace
GiacomoPope Jan 30, 2024
450b0cd
More comments and refactoring of any_root
GiacomoPope Jan 31, 2024
6b67582
Modify doctests to handle that any_root() is no longer deterministic
GiacomoPope Jan 31, 2024
cd20de4
Reintroduce traceback doctests after getting advice
GiacomoPope Feb 1, 2024
a5b5403
Apply suggestions from code review
GiacomoPope Feb 1, 2024
713ddf0
Add reviewer suggestions
GiacomoPope Feb 1, 2024
51b733c
Reformat INPUT for docstring
GiacomoPope Feb 1, 2024
89b8a4a
Clarify TODO about 37118
GiacomoPope Feb 1, 2024
bc13fa5
Clarify degree=None
GiacomoPope Feb 1, 2024
6b633fc
Clean up examples
GiacomoPope Feb 1, 2024
32baddd
Clarify TODO in CZ function
GiacomoPope Feb 1, 2024
ba8eeef
Fix failing doctests
GiacomoPope Feb 2, 2024
c37baf9
Remove some random tags from doctests and fix reported bug
GiacomoPope Feb 5, 2024
888eb3f
Merge branch 'sagemath:develop' into fix_any_root_even_char
GiacomoPope Feb 5, 2024
490ffcb
More robust doctests
GiacomoPope Feb 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 33 additions & 21 deletions src/sage/rings/polynomial/polynomial_element.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -2285,27 +2285,39 @@ cdef class Polynomial(CommutativePolynomial):
# now self has only roots of degree ``degree``.
# for now, we only implement the Cantor-Zassenhaus split
k = self.degree() // degree
if k == 1:
try:
return self.roots(ring, multiplicities=False)[0] # is there something better to do here?
except IndexError:
raise ValueError("no roots F %s" % self)
if q % 2 == 0:
while True:
T = R.random_element(2*degree-1)
if T == 0:
continue
T = T.monic()
C = T
for i in range(degree-1):
C = T + pow(C,q,self)
h = self.gcd(C)
hd = h.degree()
if hd != 0 and hd != self.degree():
if 2*hd <= self.degree():
return h.any_root(ring, -degree, True)
else:
return (self//h).any_root(ring, -degree, True)
if k == 1 or q % 2 == 0:
# Is there something better to do here for k = 1?
roots = self.roots(ring, multiplicities=False)
if roots:
from sage.misc.prandom import choice
return choice(roots)
else:
raise ValueError(f"no roots F {self}" % self)
GiacomoPope marked this conversation as resolved.
Show resolved Hide resolved
# Giacomo Pope (26-1-2024):
# This is buggy and very slow, I would like to fix it, but as far
# as I can tell, calling roots for GF(2^k) and picking one is simply
# faster and more robust.
# if q % 2 == 0:
# while True:
# T = R.random_element(2*degree-1)
# if T == 0:
# continue
# T = T.monic()
# # Compute the trace of T with field of order 2^k
# # sum T^(2^i) for i in range (degree * k)
# # We use repeated squaring to avoid redundent multiplications
# C = T
# TT = T
# for _ in range(degree * self.base_ring().degree() - 1):
# TT = pow(TT, 2, self)
# C += TT
# h = self.gcd(C)
# hd = h.degree()
# if hd != 0 and hd != self.degree():
# if 2*hd <= self.degree():
# return h.any_root(ring, -degree, True)
# else:
# return (self//h).any_root(ring, -degree, True)
else:
while True:
T = R.random_element(2*degree-1)
Expand Down
Loading