-
-
Notifications
You must be signed in to change notification settings - Fork 453
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
make charpoly/minpoly of number field elements use matrix() #5213
Comments
comment:1
Attachment: trac_5213-number-field-element-charpoly.patch.gz The patch speeds up computations of charpoly and minpoly (a lot -- sage's linear algebra is much better than pari's for some of my examples). It also makes charpoly, minpoly, absolute_charpoly, and absolute_minpoly work for relative order elements. |
comment:2
With this patch applied to my current 3.3.rc0 merge tree all doctests pass. Nick: It would be nice to have performance numbers before and after to motivate someone to review this ticket :) Cheers, Michael |
comment:3
Or perhaps not -- now it seems like this is significantly slower! All I know is that my code would not return from a charpoly call and when I changed this it was much faster. Now I can't replicate it. I will try to split the doctests and order stuff out from the actual change and figure out what I was seeing. |
comment:4
Actually, on my laptop, with the example
I get, before the patch:
and after the patch:
Just for the heck of it, I tried a degree 61 extension, and I get 7.44 seconds before the patch, and 0.26 seconds after it. So I'm motivated to review it :) Nick, if you're reading this, do you have an example where the new code is significantly slower? |
comment:5
Some experiments indicate that pari is faster (sometimes significantly) than sage for degrees up to about 20, after which sage becomes significantly faster than pari. I am attaching a .sage file that has a tuning function in it. Use it as follows:
I hope the output is fairly self-explanatory (the times are in seconds). The above run, on which it appears that degree=21 is a good point to switch algorithms, was on my laptop:
Michael said he'd run the tuning on a lot of architectures, so we can pick a switch point for the algorithms. |
comment:6
Attachment: tune_charpoly_nf.sage.gz Note: the tuning function doesn't work properly on sage-3.2.3 because it relies on K.random_element() which was only introduced recently. Running it on 3.3.rc0 is fine. |
comment:7
Here are raw number from SkyNet using 3.3.a6: x86-64:
Itanium:
Sparc:
x86:
G5:
|
comment:8
OK, we're going with a threshold degree of 25. I'm giving Nick's patch a positive review, and someone needs to look at my additional patch which implements the threshold and adds docstrings/doctests. |
comment:9
Attachment: trac_5213-treshold.patch.gz I read trac_5213-treshold.patch and give it a positive review. The doctests also pass. Cheers, Michael |
comment:10
Merged in Sage 3.3.rc1. Cheers, Michael |
Sage computes charpoly's of large matrices much faster than Pari does.
Component: number theory
Keywords: number field element minpoly charpoly
Issue created by migration from https://trac.sagemath.org/ticket/5213
The text was updated successfully, but these errors were encountered: