speedup elliptic surfaces #3884
Conversation
| @@ -899,8 +900,27 @@ function maximal_associated_points( | |||
| end | |||
| end | |||
|
|
|||
| if mode==:save_gluings && length(comps)==1 | |||
| R = radical(I) | |||
There was a problem hiding this comment.
I assume that radical first checks whether the ideal sheaf is prime and then does not compute anything, right? So this is about storing the right attributes.
There was a problem hiding this comment.
Not quite. Since length(comps)==1, the support of I is irreducible. Therefore the radical of I is prime.
There was a problem hiding this comment.
The maths was clear from the beginning.
From the comment below I deduce that this is to move it to the type RadicalIdealSheaf, which is fine with me and confirms my guess (but in a slightly different way). I would still set is_prime first and then do the radical -- and make sure that is_prime is checked and transferred to the new object by radical, if applicable.
There was a problem hiding this comment.
I was wondering whether the decomposition leading to comp did not produce further local representatives of the prime ideal sheaf already . Maybe it's worth caching those results in the RadicalIdealSheaf already?
There was a problem hiding this comment.
If we have the space for it....
| @@ -899,8 +900,27 @@ function maximal_associated_points( | |||
| end | |||
| end | |||
|
|
|||
| if mode==:save_gluings && length(comps)==1 | |||
| R = radical(I) | |||
| set_attribute!(R, :is_prime=>true) | |||
There was a problem hiding this comment.
Shouldn't this be set automatically for PrimeIdealSheafFromChart? Shouldn't this be inherited through radical in this case?
There was a problem hiding this comment.
radical(I) will be a RadicalIdealSheaf and therefore the type does not know that it is prime.
| @@ -948,15 +948,46 @@ end | |||
|
|
|||
| Return the fiber components of the fiber over the point $P \in C$. | |||
| """ | |||
| function fiber_components(S::EllipticSurface, P) | |||
| function fiber_components(S::EllipticSurface, P; algorithm=:exceptional_divisors) | |||
There was a problem hiding this comment.
Don't you want to state what type P should have? You expect it to be of a certain form later on.
If this P is an automatically generated object from further up, then you might want to give it an attribute which allows you a sanity check inside the method before continuing.
There was a problem hiding this comment.
P should be a rational point on a covered scheme. But there is no type/interface for these yet. There is a similar mess in other places.
There was a problem hiding this comment.
ok. This needs to be cleaned up at some point, but definitely not now. I agree.
* speedup elliptic surfaces (cherry picked from commit 09c52c1)
50x Speedup. Therefore please backport.
The speedup is relying on the lazy data structures of @HechtiDerLachs
which are now further exploited.