-
-
Notifications
You must be signed in to change notification settings - Fork 517
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
A previously fixed regression has reappeared #15792
Comments
This comment has been minimized.
This comment has been minimized.
comment:2
An amazing amount of time is spent for the initialisation of free modules when computing |
comment:3
These are all the tests from #11900:
|
comment:4
Aha! Very good news. At #11900, an incomplete category initialisation has been introduced for matrix spaces, to prevent some regression. But I just tested: If we now always fully initialise the matrix space, we get
In other words, it seems that we can now afford full category initialisation!! That said, we should find other ways to speed up the |
comment:5
OK, these have not been all tests from #11900. Here is another one. Without full init:
With full init it becomes 21.34 s. But that's WAAAAAY slower than the time stated in #11900. So, we have a regression, but it seems we wouldn't make things worse by a full category initialisation of matrix spaces. |
comment:6
So, the plan is: Remove the ugly hack and finally do a full initialisation of matrix spaces. And then see why everything is so slow. |
comment:7
Adding John to Cc, since the examples seemingly showing a regression since #11900 belong (I think) to his field of expertise. |
comment:8
There's another open ticket on integer points at #1-973 which I am working on when I have time. I don't have time to look at this now but it seems fomr the above comments that something has changed outside the ellipticv curve code, is that right? |
comment:9
Replying to @JohnCremona:
I suppose you mean #10973.
Not necessarily. The story is: #9138 has introduced a regression in various examples (among them integer points). Then, I fixed the regression at #11900. But since then, it seems that it got slower again. No idea what ticket was responsible, and not even what part of Sage was responsible. Actually it is not even totally clear that there is a regression, since I tested on different machines. All what I can say is: Several benchmark tests from #11900 are as fast now (on my laptop) as they used to be post-#11900 (on some desktop computer), and that's why I think the two machines are more or less comparable. And in the integer points, discriminant and gens example the timings are now much worse. I am sorry for this very long post containing almost a complete prun output for the integer_points example. But I could not find anything strange going on and can only hope that you can lay your finger on the problem. Some functions take much time, but they belong to elliptic curves ("saturate" and so on) and thus are entitled to occur in this example. However, I don't see any suspicious category or matrix operation taking an unreasonable amount of time. That's the difference to #11900, where the problem was easy to locate. Do you see more than I in the prun output?
|
comment:10
That looks completely crazily over the top for an integer points computation. Why qqbar? Why abelian groups? I will look but it will take time and I have less than none for a while. |
This comment has been minimized.
This comment has been minimized.
comment:11
There also is a regression in a different example (unrelated with elliptic curves, but part of #11900). |
This comment has been minimized.
This comment has been minimized.
comment:13
Note that in the second example from the ticket description, finite fields are involved. But their creation (in a freshly started Sage session, of course) is fast:
|
comment:14
This one might be slower than in the good old times:
|
comment:15
There is (again) some issue with overhead of the category framework:
So, creation of dynamic classes and computation of the join of categories takes a considerable amount of time. Nicolas, do you think that this issue would vanish with your "more functorial constructions"? Well, I am checking it now... |
comment:16
#10963 makes the situation even worse. I am commenting there. |
comment:19
Bump. |
comment:20
The large contribution from
so I expect that this unfortunate artifact was fixed by #15801. Looking at the present profile, as well as the runsnake boxchart, I don't see anything jumping out as problematic. Close? |
comment:21
Replying to @nbruin:
What is runsnake boxchart? If you think that the regression has disappeared, I wouldn't mind to get this ticket closed (with you as reviewer, since you did tests). |
comment:22
Replying to @simon-king-jena:
See #14414. runsnake is a graphic python profile data visualization program that makes a chart where each routine is represented by a box, and the area of each box is proportionate to the time spent. Call relations are visualized by containment of boxes. It sometimes gives a little better info than just a plain profile list (partially because the cumulative/total dilemma is taken away in a natural way). Since runsnake just needs a dump of profile data, it's sufficient to install runsnake in your system python (sage's python is a little hobbled on the graphic interface side). Anyway, we could leave this ticket open for when a new profile illustrates the reoccurrence of some regression, but it feels good to close tickets, so let's do that. |
Reviewer: Volker Braun |
In #11900, some regressions were fixed that have been introduced in #9138. It seems that one of the examples from #11900 shows a regression again:
And another example:
With the current master, both examples take twice as much time as it used to be pre-#9138 and post-#11900. Alright, it's a different computer. But still, there seems to be an issue.
CC: @nthiery @nbruin @JohnCremona
Component: performance
Reviewer: Volker Braun
Issue created by migration from https://trac.sagemath.org/ticket/15792
The text was updated successfully, but these errors were encountered: