Skip to content

Commit

Permalink
gh-37170: Total refactor of any_root() to solve issue in characteri…
Browse files Browse the repository at this point in the history 
…stic two and clean up the code

    
This PR was inspired by the issue #37160, which found that computing
isomorphisms between fields of even characteristic was taking much much
longer than expected (20s vs 100ms).

My first attempt (see commit history) were to simply patch up the
`any_root()` function, but this yielded some results which left the code
in an even more confusing state.

Following the advice of the reviewers, I have now implemented
`any_irreducible_factor()` which given a polynomial element computes an
irreducible factor of this polynomial with minimal degree. When the
optional parameter `degree=None` is set, then an irreducible factor of
degree `degree` is computed and a `ValueError` is raise if no such
factor exists.

Now, `any_root()` becomes simply a call to `self.
any_irreducible_factor(degree=1)` and a root is found from this linear
polynomial. We also handle all the other cases of optional arguments
handled by the old function, so the function *should* behave as before,
but with a cleaner code to read.

### Before PR

```python
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 10.2, Release Date: 2023-12-03                    │
│ Using Python 3.11.4. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
sage: set_random_seed(0)
....: K.<z8> = GF(2^8)
....: R.<x> = K[]
....: u = R.irreducible_element(2)
....: K_ext = K.extension(modulus=u, names="a")
....: R_ext.<y_ext> = K_ext[]
....: poly = R_ext.random_element(2).monic()
....: %time poly.roots()
CPU times: user 20.5 s, sys: 19.9 ms, total: 20.5 s
Wall time: 20.5 s
[]
```

### After PR

```py
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 10.3.beta6, Release Date: 2024-01-21              │
│ Using Python 3.11.4. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
sage: set_random_seed(0)
....: K.<z8> = GF(2^8)
....: R.<x> = K[]
....: u = R.irreducible_element(2)
....: K_ext = K.extension(modulus=u, names="a")
....: R_ext.<y_ext> = K_ext[]
....: %time R_ext.random_element(2).roots()
CPU times: user 110 ms, sys: 9.03 ms, total: 119 ms
Wall time: 150 ms
[]
```
This fixes #37160 but i think more feedback and thorough doctests need
to be included considering this is a very old function which many
projects will be using.
    
URL: #37170
Reported by: Giacomo Pope
Reviewer(s): Giacomo Pope, grhkm21, Lorenz Panny, Volker Braun
  • Loading branch information
Release Manager committed Feb 11, 2024
2 parents 4950511 + 490ffcb commit 7c0a5ec
Show file tree
Hide file tree
Showing 9 changed files with 624 additions and 304 deletions.
24 changes: 12 additions & 12 deletions src/sage/rings/finite_rings/conway_polynomials.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class PseudoConwayLattice(WithEqualityById, SageObject):
sage: # needs sage.rings.finite_rings
sage: from sage.rings.finite_rings.conway_polynomials import PseudoConwayLattice
sage: PCL = PseudoConwayLattice(2, use_database=False)
sage: PCL.polynomial(3)
sage: PCL.polynomial(3) # random
x^3 + x + 1
TESTS::
Expand Down Expand Up @@ -164,16 +164,16 @@ def __init__(self, p, use_database=True):
sage: # needs sage.rings.finite_rings
sage: from sage.rings.finite_rings.conway_polynomials import PseudoConwayLattice
sage: PCL = PseudoConwayLattice(3)
sage: PCL.polynomial(3)
sage: PCL.polynomial(3) # random
x^3 + 2*x + 1
sage: # needs sage.rings.finite_rings
sage: PCL = PseudoConwayLattice(5, use_database=False)
sage: PCL.polynomial(12)
sage: PCL.polynomial(12) # random
x^12 + 4*x^11 + 2*x^10 + 4*x^9 + 2*x^8 + 2*x^7 + 4*x^6 + x^5 + 2*x^4 + 2*x^2 + x + 2
sage: PCL.polynomial(6)
sage: PCL.polynomial(6) # random
x^6 + x^5 + 4*x^4 + 3*x^3 + 3*x^2 + 2*x + 2
sage: PCL.polynomial(11)
sage: PCL.polynomial(11) # random
x^11 + x^6 + 3*x^3 + 4*x + 3
"""
self.p = p
Expand Down Expand Up @@ -215,11 +215,11 @@ def polynomial(self, n):
sage: # needs sage.rings.finite_rings
sage: from sage.rings.finite_rings.conway_polynomials import PseudoConwayLattice
sage: PCL = PseudoConwayLattice(2, use_database=False)
sage: PCL.polynomial(3)
sage: PCL.polynomial(3) # random
x^3 + x + 1
sage: PCL.polynomial(4)
sage: PCL.polynomial(4) # random
x^4 + x^3 + 1
sage: PCL.polynomial(60)
sage: PCL.polynomial(60) # random
x^60 + x^59 + x^58 + x^55 + x^54 + x^53 + x^52 + x^51 + x^48 + x^46 + x^45 + x^42 + x^41 + x^39 + x^38 + x^37 + x^35 + x^32 + x^31 + x^30 + x^28 + x^24 + x^22 + x^21 + x^18 + x^17 + x^16 + x^15 + x^14 + x^10 + x^8 + x^7 + x^5 + x^3 + x^2 + x + 1
"""
if n in self.nodes:
Expand All @@ -239,7 +239,7 @@ def polynomial(self, n):
# TODO: something like the following
# gcds = [n.gcd(d) for d in self.nodes.keys()]
# xi = { m: (...) for m in gcds }
xi = {q: self.polynomial(n//q).any_root(K, -n//q, assume_squarefree=True)
xi = {q: self.polynomial(n//q).any_root(K, n//q, assume_squarefree=True, assume_distinct_deg=True)
for q in n.prime_divisors()}

# The following is needed to ensure that in the concrete instantiation
Expand Down Expand Up @@ -402,9 +402,9 @@ def _frobenius_shift(K, generators, check_only=False):
sage: f20 = x^20 + x^19 + x^15 + x^13 + x^12 + x^11 + x^9 + x^8 + x^7 + x^4 + x^2 + x + 1
sage: f12 = x^12 + x^10 + x^9 + x^8 + x^4 + x^2 + 1
sage: K.<a> = GF(2^60, modulus='first_lexicographic')
sage: x30 = f30.any_root(K)
sage: x20 = f20.any_root(K)
sage: x12 = f12.any_root(K)
sage: x30 = f30.roots(K, multiplicities=False)[0]
sage: x20 = f20.roots(K, multiplicities=False)[0]
sage: x12 = f12.roots(K, multiplicities=False)[0]
sage: generators = {2: x30, 3: x20, 5: x12}
sage: from sage.rings.finite_rings.conway_polynomials import _frobenius_shift, _find_pow_of_frobenius
sage: _frobenius_shift(K, generators)
Expand Down
50 changes: 31 additions & 19 deletions src/sage/rings/finite_rings/hom_finite_field.pyx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,32 +16,35 @@ Construction of an embedding::
sage: k.<t> = GF(3^7)
sage: K.<T> = GF(3^21)
sage: f = FiniteFieldHomomorphism_generic(Hom(k, K)); f
sage: f = FiniteFieldHomomorphism_generic(Hom(k, K)); f # random
Ring morphism:
From: Finite Field in t of size 3^7
To: Finite Field in T of size 3^21
Defn: t |--> T^20 + 2*T^18 + T^16 + 2*T^13 + T^9 + 2*T^8 + T^7 + T^6 + T^5 + T^3 + 2*T^2 + T
sage: f(t)
sage: f(t) # random
T^20 + 2*T^18 + T^16 + 2*T^13 + T^9 + 2*T^8 + T^7 + T^6 + T^5 + T^3 + 2*T^2 + T
sage: f(t) == f.im_gens()[0]
True
The map `f` has a method ``section`` which returns a partially defined
map which is the inverse of `f` on the image of `f`::
sage: g = f.section(); g
sage: g = f.section(); g # random
Section of Ring morphism:
From: Finite Field in t of size 3^7
To: Finite Field in T of size 3^21
Defn: t |--> T^20 + 2*T^18 + T^16 + 2*T^13 + T^9 + 2*T^8 + T^7 + T^6 + T^5 + T^3 + 2*T^2 + T
sage: g(f(t^3+t^2+1))
t^3 + t^2 + 1
sage: a = k.random_element()
sage: g(f(a)) == a
True
sage: g(T)
Traceback (most recent call last):
...
ValueError: T is not in the image of Ring morphism:
From: Finite Field in t of size 3^7
To: Finite Field in T of size 3^21
Defn: t |--> T^20 + 2*T^18 + T^16 + 2*T^13 + T^9 + 2*T^8 + T^7 + T^6 + T^5 + T^3 + 2*T^2 + T
Defn: ...
There is no embedding of `GF(5^6)` into `GF(5^11)`::
Expand Down Expand Up @@ -130,16 +133,17 @@ cdef class SectionFiniteFieldHomomorphism_generic(Section):
sage: K.<T> = GF(3^21)
sage: f = FiniteFieldHomomorphism_generic(Hom(k, K))
sage: g = f.section()
sage: g(f(t^3+t^2+1))
t^3 + t^2 + 1
sage: a = k.random_element()
sage: g(f(a)) == a
True
sage: g(T)
Traceback (most recent call last):
...
ValueError: T is not in the image of Ring morphism:
From: Finite Field in t of size 3^7
To: Finite Field in T of size 3^21
Defn: t |--> T^20 + 2*T^18 + T^16 + 2*T^13 + T^9 + 2*T^8 + T^7 + T^6 + T^5 + T^3 + 2*T^2 + T
Defn: t |--> ...
"""
for root, _ in x.minimal_polynomial().roots(ring=self.codomain()):
if self._inverse(root) == x:
Expand All @@ -158,7 +162,7 @@ cdef class SectionFiniteFieldHomomorphism_generic(Section):
sage: K.<T> = GF(3^21)
sage: f = FiniteFieldHomomorphism_generic(Hom(k, K))
sage: g = f.section()
sage: g._repr_()
sage: g._repr_() # random
'Section of Ring morphism:\n From: Finite Field in t of size 3^7\n To: Finite Field in T of size 3^21\n Defn: t |--> T^20 + 2*T^18 + T^16 + 2*T^13 + T^9 + 2*T^8 + T^7 + T^6 + T^5 + T^3 + 2*T^2 + T'
"""
return "Section of %s" % self._inverse
Expand Down Expand Up @@ -202,11 +206,15 @@ cdef class FiniteFieldHomomorphism_generic(RingHomomorphism_im_gens):
sage: from sage.rings.finite_rings.hom_finite_field import FiniteFieldHomomorphism_generic
sage: k.<t> = GF(3^7)
sage: K.<T> = GF(3^21)
sage: f = FiniteFieldHomomorphism_generic(Hom(k, K)); f
sage: f = FiniteFieldHomomorphism_generic(Hom(k, K)); f # random
Ring morphism:
From: Finite Field in t of size 3^7
To: Finite Field in T of size 3^21
Defn: t |--> T^20 + 2*T^18 + T^16 + 2*T^13 + T^9 + 2*T^8 + T^7 + T^6 + T^5 + T^3 + 2*T^2 + T
sage: a = k.random_element()
sage: b = k.random_element()
sage: f(a) + f(b) == f(a + b)
True
sage: k.<t> = GF(3^6)
sage: K.<t> = GF(3^9)
Expand Down Expand Up @@ -299,7 +307,7 @@ cdef class FiniteFieldHomomorphism_generic(RingHomomorphism_im_gens):
sage: k.<t> = GF(3^3)
sage: K.<T> = GF(3^9)
sage: f = FiniteFieldHomomorphism_generic(Hom(k, K))
sage: f(t)
sage: f(t) # random
2*T^6 + 2*T^4 + T^2 + T
sage: a = k.random_element()
Expand Down Expand Up @@ -368,20 +376,22 @@ cdef class FiniteFieldHomomorphism_generic(RingHomomorphism_im_gens):
sage: k.<t> = GF(3^7)
sage: K.<T> = GF(3^21)
sage: f = FiniteFieldHomomorphism_generic(Hom(k, K))
sage: g = f.section(); g
sage: g = f.section(); g # random
Section of Ring morphism:
From: Finite Field in t of size 3^7
To: Finite Field in T of size 3^21
Defn: t |--> T^20 + 2*T^18 + T^16 + 2*T^13 + T^9 + 2*T^8 + T^7 + T^6 + T^5 + T^3 + 2*T^2 + T
sage: g(f(t^3+t^2+1))
t^3 + t^2 + 1
sage: a = k.random_element()
sage: b = k.random_element()
sage: g(f(a) + f(b)) == a + b
True
sage: g(T)
Traceback (most recent call last):
...
ValueError: T is not in the image of Ring morphism:
From: Finite Field in t of size 3^7
To: Finite Field in T of size 3^21
Defn: t |--> T^20 + 2*T^18 + T^16 + 2*T^13 + T^9 + 2*T^8 + T^7 + T^6 + T^5 + T^3 + 2*T^2 + T
Defn: ...
"""
if self.base_map() is not None:
raise NotImplementedError
Expand Down Expand Up @@ -421,7 +431,7 @@ cdef class FiniteFieldHomomorphism_generic(RingHomomorphism_im_gens):
sage: k.<t> = GF(5^3)
sage: Frob = k.frobenius_endomorphism()
sage: embed = Frob.fixed_field()[1]
sage: hash(embed) # random
sage: hash(embed) # random
-2441354824160407762
"""
return Morphism.__hash__(self)
Expand Down Expand Up @@ -751,15 +761,17 @@ cdef class FrobeniusEndomorphism_finite_field(FrobeniusEndomorphism_generic):
sage: kfixed, embed = f.fixed_field()
sage: kfixed
Finite Field in t_fixed of size 5^2
sage: embed
sage: embed # random
Ring morphism:
From: Finite Field in t_fixed of size 5^2
To: Finite Field in t of size 5^6
Defn: t_fixed |--> 4*t^5 + 2*t^4 + 4*t^2 + t
sage: tfixed = kfixed.gen()
sage: embed(tfixed)
sage: embed(tfixed) # random
4*t^5 + 2*t^4 + 4*t^2 + t
sage: embed(tfixed) == embed.im_gens()[0]
True
"""
if self._degree_fixed == 1:
k = FiniteField(self.domain().characteristic())
Expand Down
22 changes: 16 additions & 6 deletions src/sage/rings/finite_rings/hom_finite_field_givaro.pyx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,15 @@ cdef class SectionFiniteFieldHomomorphism_givaro(SectionFiniteFieldHomomorphism_
sage: k.<t> = GF(3^2)
sage: K.<T> = GF(3^4)
sage: f = FiniteFieldHomomorphism_givaro(Hom(k, K))
sage: g = f.section(); g
sage: g = f.section(); g # random
Section of Ring morphism:
From: Finite Field in t of size 3^2
To: Finite Field in T of size 3^4
Defn: t |--> 2*T^3 + 2*T^2 + 1
sage: a = k.random_element()
sage: b = k.random_element()
sage: g(f(a) + f(b)) == g(f(a)) + g(f(b)) == a + b
True
"""
if not isinstance(inverse, FiniteFieldHomomorphism_givaro):
raise TypeError("The given map is not an instance of FiniteFieldHomomorphism_givaro")
Expand Down Expand Up @@ -114,7 +118,7 @@ cdef class SectionFiniteFieldHomomorphism_givaro(SectionFiniteFieldHomomorphism_
ValueError: T is not in the image of Ring morphism:
From: Finite Field in t of size 3^2
To: Finite Field in T of size 3^4
Defn: t |--> 2*T^3 + 2*T^2 + 1
Defn: t |--> ...
"""
if x.parent() != self.domain():
raise TypeError("%s is not in %s" % (x, self.domain()))
Expand All @@ -140,11 +144,15 @@ cdef class FiniteFieldHomomorphism_givaro(FiniteFieldHomomorphism_generic):
sage: from sage.rings.finite_rings.hom_finite_field_givaro import FiniteFieldHomomorphism_givaro
sage: k.<t> = GF(3^2)
sage: K.<T> = GF(3^4)
sage: f = FiniteFieldHomomorphism_givaro(Hom(k, K)); f
sage: f = FiniteFieldHomomorphism_givaro(Hom(k, K)); f # random
Ring morphism:
From: Finite Field in t of size 3^2
To: Finite Field in T of size 3^4
Defn: t |--> 2*T^3 + 2*T^2 + 1
sage: a = k.random_element()
sage: b = k.random_element()
sage: f(a) + f(b) == f(a + b)
True
sage: k.<t> = GF(3^10)
sage: K.<T> = GF(3^20)
Expand Down Expand Up @@ -182,8 +190,10 @@ cdef class FiniteFieldHomomorphism_givaro(FiniteFieldHomomorphism_generic):
sage: k.<t> = GF(3^2)
sage: K.<T> = GF(3^4)
sage: f = FiniteFieldHomomorphism_givaro(Hom(k, K))
sage: f(t)
sage: f(t) # random
2*T^3 + 2*T^2 + 1
sage: f(t) == f.im_gens()[0]
True
"""
if x.parent() != self.domain():
raise TypeError("%s is not in %s" % (x, self.domain()))
Expand Down Expand Up @@ -242,14 +252,14 @@ cdef class FrobeniusEndomorphism_givaro(FrobeniusEndomorphism_finite_field):
sage: kfixed, embed = f.fixed_field()
sage: kfixed
Finite Field in t_fixed of size 5^2
sage: embed
sage: embed # random
Ring morphism:
From: Finite Field in t_fixed of size 5^2
To: Finite Field in t of size 5^6
Defn: t_fixed |--> 4*t^5 + 2*t^4 + 4*t^2 + t
sage: tfixed = kfixed.gen()
sage: embed(tfixed)
sage: embed(tfixed) # random
4*t^5 + 2*t^4 + 4*t^2 + t
"""
if self._degree_fixed == 1:
Expand Down
2 changes: 1 addition & 1 deletion src/sage/rings/finite_rings/homset.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ def _an_element_(self):
TESTS::
sage: Hom(GF(3^3, 'a'), GF(3^6, 'b')).an_element()
sage: Hom(GF(3^3, 'a'), GF(3^6, 'b')).an_element() # random
Ring morphism:
From: Finite Field in a of size 3^3
To: Finite Field in b of size 3^6
Expand Down
Loading

0 comments on commit 7c0a5ec

Please sign in to comment.