-
-
Notifications
You must be signed in to change notification settings - Fork 488
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
is_prime for ideals uses factorization, can be VERY slow #33360
Comments
comment:1
Tracked this down to
Behind the scenes this is handled by pari as follows:
It is possible to use pari to test for primality much faster without factoring as follows:
I'll try to cook up a patch for Another issue: during the call to |
comment:2
Ctrl-C works for me (I instantaneously get |
comment:3
For the record:
|
comment:5
|
Branch: u/tornaria/33360-ideal.is_prime |
comment:7
To fix the issue, we only factor the ideal if the norm is a prime power. Note that a simpler way would be to use pari
However it turns out that
This is fixed in current development version of pari by this commit. New commits:
|
Author: Gonzalo Tornaría |
Commit: |
This comment has been minimized.
This comment has been minimized.
comment:9
The patch looks good as far as I can tell. Just one question: How much faster is |
Reviewer: Lorenz Panny |
comment:10
Replying to @yyyyx4:
I doubt the difference is significative. For the example in the doctest:
which probably just means pari is optimizing in a different way but that will depend on the ideal. In the long run, I think using Using For a prime ideal, I guess the time would be dominated by |
comment:11
The Example: R.<x> = QQ[]
K.<a,b,c> = NumberField([x^2-2,x^2-3,x^2-5])
Kpari = K.pari_nf()
for bits in (50, 100, 200, 300, 500, 800):
p = random_prime(2^bits, proof=False)
I = choice([f for f,_ in K.ideal(p).factor()]) # a prime above p
Ipari = I.pari_hnf()
print(f'{bits} bits')
%timeit Kpari.idealismaximal(Ipari)
%timeit assert Kpari.idealnorm(Ipari).isprimepower(); Kpari.idealfactor(Ipari) Result:
|
comment:12
We have to be careful with Also, a small optimization: Calling |
Changed author from Gonzalo Tornaría to Gonzalo Tornaría, Lorenz Panny |
comment:15
I implemented my suggestions from comment:9 and comment:12. Someone else will have to review the new changes. New commits:
|
Changed branch from u/tornaria/33360-ideal.is_prime to public/33360 |
Dependencies: #34537 |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:20
Reverted earlier changes from comment:16 as #34537 appears to be stuck. Can we get this in? |
Changed dependencies from #34537 to none |
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Removed branch from issue description; replaced by PR #34980 |
The implementation of
is_prime()
for ideals uses pariidealfactor()
. This is a bad idea if the norm of the ideal is hard to factor.Pari has
ismaximalideal()
, but it is broken in pari 2.13.3. As a workaround, since a prime ideal has prime power norm, we avoid factorization except on this (easy) case.Was: VERY slow doctest in
sage/rings/number_field/number_field.py
With sage 9.5:
CC: @yyyyx4 @fchapoton
Component: number fields
Author: Gonzalo Tornaría, Lorenz Panny
Reviewer: Lorenz Panny
Issue created by migration from https://trac.sagemath.org/ticket/33360
The text was updated successfully, but these errors were encountered: