Add additional bindings from NTL to Polynomial_ZZ_pEX

[GiacomoPope]

There are current cython bindings to NTL methods which are not made available to polynomial rings over extension fields.

For some of these bindings, their introduction does not offer much speed up, but there is a 50x improvement for using NTL reverse over that of the current implementation, and about a 20% speed up using the NTL ZZ_pEX_InvTrunc over what is currently implemented.

Method names and functionalities have been preserved, so this simply improves performance for the Polynomial_ZZ_pEX type without effecting other polynomial ring classes.

Before Patch

sage: p = next_prime(2**1024)
sage: F.<i> = GF(p^2)
sage: R.<x> = PolynomialRing(F, implementation="NTL")
sage:
sage: f = R([p for p in primes(10)])
sage: %timeit f.reverse()
125 µs ± 582 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
sage: %timeit f.inverse_series_trunc(500)
4.96 ms ± 81.3 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
sage:
sage: f = R([p for p in primes(100)])
sage: %timeit f.reverse()
681 µs ± 4.21 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
sage: %timeit f.inverse_series_trunc(500)
7.1 ms ± 62.8 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
sage:
sage: f = R([p for p in primes(1000)])
sage: %timeit f.reverse()
4.57 ms ± 77 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
sage: %timeit f.inverse_series_trunc(500)
10.9 ms ± 82.7 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

After Patch

sage: p = next_prime(2**1024)
sage: F.<i> = GF(p^2)
sage: R.<x> = PolynomialRing(F, implementation="NTL")
sage: 
sage: f = R([p for p in primes(10)])
sage: %timeit f.reverse()
836 ns ± 9.91 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
sage: %timeit f.inverse_series_trunc(500)
1.84 ms ± 77.1 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
sage: 
sage: f = R([p for p in primes(100)])
sage: %timeit f.reverse()
3.63 µs ± 103 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)
sage: %timeit f.inverse_series_trunc(500)
6.66 ms ± 441 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

sage: f = R([p for p in primes(1000)])
sage: %timeit f.reverse()
26.8 µs ± 1.48 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
sage: %timeit f.inverse_series_trunc(500)
8.75 ms ± 432 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

📝 Checklist

• The title is concise, informative, and self-explanatory.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion.
• I have created tests covering the changes.
• I have updated the documentation accordingly.

⌛ Dependencies

The only additional import is sig_on() and sig_off() into the Polynomial_ZZ_pEX.pyx

Warning!

This is my first attempt to contribute new functions to sagemath, and it's also my first few weeks of playing with cython. I have done my best to do what is needed, but maybe I have done something incorrect. Please let me know if there's more I can do.

In particular, I know an alternative edit to the one I proposed is to modify the child Polynomial_template class instead of the Polynomial_ZZ_pEX class, but I know less about all the other parents of this class and I was concerned I may make breaking changes somewhere downstream.

Currently, this just adds the NTL methods when I know they are avaialble and they simply do what is done currently, but faster.

[fchapoton]

• the linter is not happyabout whitespaces
• the "raise" introduced need to be doctested

[GiacomoPope]

Tried to fix all the pycodestyle problems, which meant editing more of the file than just my additions, hopefully this is OK, but please let me know.

I added doctests for the two functions I've included but noticed other functions in this file are missing doctests.

[fchapoton]

Thanks. You are only responsible for the code that you want to add.

[fchapoton]

cython files are not supposed to be fully compliant with pycodestyle. You should rather undo your pycodestyle changes, except the ones insise your new code.

[GiacomoPope]

Thank you for your advice. I have reverted the changes against old code and tidied up my own to match (hopefully with linting remaining happy)

[fchapoton]

Thanks. The first line of each traceback doctest result should rather be
Traceback (most recent call last):

[GiacomoPope]

Before committing etc, I will check. For interactive sage I have:

ValueError                                Traceback (most recent call last)
Cell In [6], line 1
----> 1 f.reverse(degree=-Integer(1))

File ~/sage/sage/src/sage/rings/polynomial/polynomial_zz_pex.pyx:537, in sage.rings.polynomial.polynomial_zz_pex.Polynomial_ZZ_pEX.reverse()
    535 if degree is not None:
    536     if degree <= 0:
--> 537         raise ValueError("degree argument must be a non-negative integer, got %s" % (degree))
    538     try:
    539         d = degree

ValueError: degree argument must be a non-negative integer, got -1

I should have only:

Traceback (most recent call last):
...
ValueError: degree argument must be a non-negative integer, got -1

Do I need the ellipse, or can I do

Traceback (most recent call last):
ValueError: degree argument must be a non-negative integer, got -1

[fchapoton]

you shoudl write with the ellipsis

Traceback (most recent call last):
...
ValueError: degree argument must be a non-negative integer, got -1

You can use "git grep -A2 Traceback src/sage" to compare with existing doctests of this shape.

[fchapoton]

there is a failing doctest in

File "sage/rings/polynomial/polynomial_zz_pex.pyx", line 510, in sage.rings.polynomial.polynomial_zz_pex.Polynomial_ZZ_pEX.reverse
Failed example:
    f.reverse(degree=10)
Expected:
    2*x^10 + 3*x^9
Got:
    4*x^10 + 32*x^4 + 11

[fchapoton]

ok, looks good. Still some little tweaks required in the doc :

In the first new method, degree should be written with two backticks before and after, not only one

In the second new method, separate the first line and the rest of the first paragraph by an empty line.

[fchapoton]

The failing doctests are known and unrelated to the present changes

[fchapoton]

ok, good to go. Thanks

[github-actions]

Documentation preview for this PR (built with commit 1308baa; changes) is ready! 🎉

[GiacomoPope]

Thanks for all your help in getting this ready. My next contribution should be much smoother as a result :)