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

establish interface for instantiated classical modular polynomials #36190

Conversation

yyyyx4
Copy link
Member

@yyyyx4 yyyyx4 commented Sep 4, 2023

Generic modular polynomials (i.e., as elements of $\mathbb Z[X,Y]$) grow very big very quickly. However, most algorithmic applications only require evaluations $\Phi_\ell(j,Y)$ of those polynomials. There are algorithms to compute such evaluations faster than computing-then-evaluating the full modular polynomial, for example due to Sutherland. PARI implements both kinds of algorithms in polmodular().

This patch adds a common interface for (1) accessing Kohel's database of modular polynomials if available, (2) calling PARI's polmodular() to compute generic modular polynomials, and (3) calling PARI's polmodular() to compute instantiated modular polynomials. Results are opportunistically cached and reused whenever this seems to make sense.

@yyyyx4 yyyyx4 force-pushed the public/interface_for_instantiated_modular_polynomials branch from 8fec3f7 to 3054650 Compare September 4, 2023 11:44
@yyyyx4 yyyyx4 marked this pull request as draft September 5, 2023 07:13
@yyyyx4 yyyyx4 force-pushed the public/interface_for_instantiated_modular_polynomials branch 2 times, most recently from 2b52f71 to a6ebb72 Compare September 5, 2023 15:34
@yyyyx4 yyyyx4 marked this pull request as ready for review September 5, 2023 15:37
src/sage/schemes/elliptic_curves/all.py Outdated Show resolved Hide resolved
src/sage/schemes/elliptic_curves/mod_poly.py Outdated Show resolved Hide resolved
src/sage/schemes/elliptic_curves/mod_poly.py Outdated Show resolved Hide resolved
src/sage/schemes/elliptic_curves/mod_poly.py Outdated Show resolved Hide resolved
src/sage/schemes/elliptic_curves/mod_poly.py Show resolved Hide resolved
src/sage/schemes/elliptic_curves/mod_poly.py Outdated Show resolved Hide resolved
src/sage/schemes/elliptic_curves/mod_poly.py Outdated Show resolved Hide resolved
src/sage/schemes/elliptic_curves/mod_poly.py Outdated Show resolved Hide resolved
@yyyyx4 yyyyx4 force-pushed the public/interface_for_instantiated_modular_polynomials branch from a6ebb72 to 2ddd0d2 Compare September 26, 2023 08:50
@yyyyx4 yyyyx4 requested a review from tscrim September 26, 2023 08:52
Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some last little things.

src/sage/schemes/elliptic_curves/mod_poly.py Outdated Show resolved Hide resolved
src/sage/schemes/elliptic_curves/mod_poly.py Outdated Show resolved Hide resolved
…_polynomials

SageMath version 10.2.beta9, Release Date: 2023-10-30
@yyyyx4 yyyyx4 requested a review from tscrim November 2, 2023 13:27
Copy link

github-actions bot commented Nov 3, 2023

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

Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>
@yyyyx4
Copy link
Member Author

yyyyx4 commented Nov 7, 2023

Done, thank you for reviewing!

@tscrim
Copy link
Collaborator

tscrim commented Nov 7, 2023

Thank you.

@JohnCremona
Copy link
Member

Thanks for implementing this!

vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 4, 2023
…lar polynomials

    
Generic modular polynomials (i.e., as elements of $\mathbb Z[X,Y]$) grow
very big very quickly. However, most algorithmic applications only
require evaluations $\Phi_\ell(j,Y)$ of those polynomials. There are
algorithms to compute such evaluations faster than computing-then-
evaluating the full modular polynomial, for example due to
[Sutherland](https://arxiv.org/abs/1202.3985). PARI implements both
kinds of algorithms in `polmodular()`.

This patch adds a common interface for (1) accessing Kohel's database of
modular polynomials if available, (2) calling PARI's `polmodular()` to
compute generic modular polynomials, and (3) calling PARI's
`polmodular()` to compute instantiated modular polynomials. Results are
opportunistically cached and reused whenever this seems to make sense.
    
URL: sagemath#36190
Reported by: Lorenz Panny
Reviewer(s): Lorenz Panny, Travis Scrimshaw
@vbraun vbraun merged commit fb7ef07 into sagemath:develop Dec 6, 2023
8 of 18 checks passed
@mkoeppe mkoeppe added this to the sage-10.3 milestone Dec 6, 2023
@yyyyx4 yyyyx4 deleted the public/interface_for_instantiated_modular_polynomials branch December 6, 2023 14:52
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 21, 2024
…olynomial`

    
There is a bug in the new `classical_modular_polynomial` function
created in sagemath#36190.

A simple example to reproduce the bug is the following

```
p = 2^216 * 3^137 - 1
F.<i> = GF(p^2, modulus=[1,0,1])
E = EllipticCurve(F, [0, 6, 0, 1, 0])
classical_modular_polynomial(2, E.j_invariant())
```

This will fail with a `TypeError`.

The bug is due to the interface with the pari function `polmodular`. In
particular, contrary to the [documentation](http://pari.math.u-
bordeaux.fr/dochtml/ref/Polynomials_and_power_series.html#polmodular),
the `polmodular` function will only evaluate $\Phi_\ell(j, Y)$ for
$j$-invariants that are defined over $\mathbb F_p$, and not over any
generic finite field.
If however `j.parent()` is a generic finite field, but `j` itself is
defined over the prime field, pari will evaluate that and the current
implementation fails to convert back to a sage polynomial.

The proposed fix is to cast the result of the pari call to `ZZ['Y']`,
since the result of `polmodular` currently returns a polynomial in the
correct $\mathbb Z/n\mathbb Z[Y]$.

#sd123
    
URL: sagemath#37094
Reported by: Riccardo Zanotto
Reviewer(s): Frédéric Chapoton, Peter Bruin, Riccardo Zanotto
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants