gh-37443: Fix bugs and regression in performance of `any_root()`
This pull request aims to fix some issues which came with the
refactoring of `any_root()` in
https://github.com/sagemath/sage/pull/37170.
I am afraid that apart from a little more cleaning up and verbose
comments, this "fix" really relies on `f.roots()` rather than
`f.any_root()` when roots over extension fields are computed.
The core problem currently is that the proper workflow should be the
following:
- Given a polynomial $f$, find an irreducible factor of smallest degree.
When `ring` is `None` and `degree` is `None`, only linear factors are
searched for. This code is unchanged and works fast.
- When `degree` is `None` and `ring` is not `None`, the old code used to
perform `f.change_ring(ring).any_root()` and look for a linear factor in
`ring`. The issue is when `ring` is a field with a large degree,
arithmetic in this ring is very slow and root finding is very slow.
- When `degree` is not `None` then an irreducible of degree `degree` is
searched for, $f$, and then a root is found in an extension ring. This
is either the user supplied `ring`, or it is in the extension
`self.base_ring().extension(degree)`. Again, as this extension could be
large, this can be slow.
Now, the reason that there's a regression in speed, as explained in
https://github.com/sagemath/sage/issues/37359, is that the method before
refactoring simply called `f.roots(ring)[0]` when working for these
extensions. As the root finding is always performed by a C library this
is much faster than the python code which we have for `any_root()` and
so the more "pure" method I wrote simply is slower.
The real issue holding this method back is that if we are in some field
$GF(p^k)$ and `ring` is some extension $GF(p^{nk})$ then we are not
guaranteed a coercion between these rings with the current coercion
system. Furthermore, if we extend the base to some $GF(p^{dk})$ with
$dk$ dividing $nk$ we also have no certainty of a coercion from this
minimal extension to the user supplied ring.
As a result, a "good" fix is delayed until we figure out finite field
coercion. **Issue to come**.
Because of all of this, this PR goes back to the old behaviour, while
keeping the refactored and easier to read code. I hope one day in the
future to fix this.
Fixes https://github.com/sagemath/sage/issues/37359
Fixes https://github.com/sagemath/sage/issues/37442
Fixes https://github.com/sagemath/sage/issues/37445
Fixes https://github.com/sagemath/sage/issues/37471
**EDIT**
I have labeled this as critical as the slow down in the most extreme
cases causes code to complete hang when looking for a root. See for
example: https://github.com/sagemath/sage/issues/37442. I do not want
https://github.com/sagemath/sage/pull/37170 to be merged into 10.3
without this patch.
Additionally, a typo meant that a bug was introduced in the CZ
splitting. This has also been fixed.
Also, also in the refactoring the function behaved as intended (rather
than the strange behaviour from before) which exposed an issue
https://github.com/sagemath/sage/issues/37471 which I have fixed.
URL: https://github.com/sagemath/sage/pull/37443
Reported by: Giacomo Pope
Reviewer(s): Giacomo Pope, grhkm21, Lorenz Panny, Travis Scrimshaw
