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.

Sign up for GitHub

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

Jump to bottom

speedup elliptic surfaces #3884

Merged
merged 2 commits into from
Jun 24, 2024
Merged

speedup elliptic surfaces #3884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a1c919
speedup elliptic surfaces
simonbrandhorst Jun 24, 2024
c07b764
fix mysterious bug
simonbrandhorst Jun 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions experimental/Schemes/src/IdealSheaves.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,8 @@ function maximal_associated_points(
I::AbsIdealSheaf;
covering=default_covering(scheme(I)),
use_decomposition_info::Bool=true,
algorithm::Symbol=:GTZ
algorithm::Symbol=:GTZ,
mode::Symbol=:save_gluings
)
X = scheme(I)
comps = AbsIdealSheaf[]
Expand Down Expand Up @@ -899,8 +900,27 @@ function maximal_associated_points(
end
end

if mode==:save_gluings && length(comps)==1
R = radical(I)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not quite. Since length(comps)==1, the support of I is irreducible. Therefore the radical of I is prime.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we have the space for it....

set_attribute!(R, :is_prime=>true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be set automatically for PrimeIdealSheafFromChart? Shouldn't this be inherited through radical in this case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

radical(I) will be a RadicalIdealSheaf and therefore the type does not know that it is prime.

Copy link
Collaborator

Choose a reason for hiding this comment

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

see above.

P = comps[1]
U = original_chart(P)
object_cache(R)[U] = P(U)
comps = AbsIdealSheaf[R]
end
# Manually fill up the cache
if !use_decomposition_info # We can only do this if all we treated all components on all charts.
if use_decomposition_info
# we can at least remember some partial information
for U in patches(covering)
if isone(I(U))
I_one = ideal(OO(U), one(OO(U)))
for P in comps
object_cache(P)[U] = I_one
end
end
end
else
# We can only do this if all we treated all components on all charts.
for U in patches(covering)
I_one = ideal(OO(U), one(OO(U)))
for P in comps
Expand Down Expand Up @@ -1390,6 +1410,7 @@ function saturation(I::AbsIdealSheaf, J::AbsIdealSheaf)
end

function pushforward(f::AbsCoveredSchemeMorphism, II::AbsIdealSheaf)
@req domain(f)===space(II) "ideal sheaf not defined on domain of morphism"
f_cov = covering_morphism(f)
dom_cov = domain(f_cov)
cod_cov = codomain(f_cov)
Expand All @@ -1401,6 +1422,7 @@ function pushforward(f::AbsCoveredSchemeMorphism, II::AbsIdealSheaf)
end
return IdealSheaf(codomain(f), ideal_dict, check=true) #TODO: Set to false
end


function Base.:^(II::AbsIdealSheaf, k::IntegerUnion)
k < 0 && error("negative powers of ideal sheaves are not allowed")
Expand Down Expand Up @@ -1933,7 +1955,7 @@ function is_subset(I::PrimeIdealSheafFromChart, rad::RadicalOfIdealSheaf)
U = original_chart(I)
return all(g->radical_membership(g, rad(U)), gens(I(U)))
end

function radical(I::AbsIdealSheaf)
result = RadicalOfIdealSheaf(I)
return result
Expand All @@ -1944,6 +1966,13 @@ is_radical(rad::RadicalOfIdealSheaf) = true
########################################################################
# custom functionality for prime ideal sheaves from chart
########################################################################
function is_subset(I::AbsIdealSheaf, P::PrimeIdealSheafFromChart)
X = scheme(I)
@assert X === scheme(P)
U = original_chart(P)
return is_subset(I(U), P(U))
end


function is_subset(P::PrimeIdealSheafFromChart, I::AbsIdealSheaf)
X = scheme(P)
Expand Down
45 changes: 38 additions & 7 deletions experimental/Schemes/src/elliptic_surface.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok. This needs to be cleaned up at some point, but definitely not now. I agree.

@vprint :EllipticSurface 2 "computing fiber components over $(P)\n"
F = fiber_cartier(S, P)
@vprint :EllipticSurface 2 "decomposing fiber "
comp = maximal_associated_points(ideal_sheaf(F))
@vprint :EllipticSurface 2 "done decomposing fiber\n"
return [weil_divisor(c, check=false) for c in comp]
P = base_ring(S).(P)
W = codomain(S.inc_Weierstrass)
Fcart = fiber_cartier(S, P)
if isone(P[2])
U = default_covering(W)[1]
(x,y,t) = coordinates(U)
F = PrimeIdealSheafFromChart(W, U, ideal(t - P[1]))
elseif isone(P[1])
U = default_covering(W)[4]
(x,y,s) = coordinates(U)
F = PrimeIdealSheafFromChart(W, U, ideal(s - P[2]))
end
FF = ideal_sheaf(Fcart)
EE = exceptional_divisors(S)
EP = filter(E->issubset(FF, E), EE)
for bl in S.ambient_blowups
F = strict_transform(bl, F)
end
F = pullback(S.inc_Y, F)
F = weil_divisor(F, ZZ)
fiber_components = [weil_divisor(E, ZZ) for E in EP]
push!(fiber_components, F)
return fiber_components
end


@attr function exceptional_divisors(S::EllipticSurface)
PP = AbsIdealSheaf[]
@vprintln :EllipticSurface 2 "computing exceptional divisors"
for E in S.ambient_exceptionals
@vprintln :EllipticSurface 4 "decomposing divisor "
mp = maximal_associated_points(ideal_sheaf(pullback(S.inc_Y,E));
use_decomposition_info=true)
append!(PP, mp)
end
@vprintln :EllipticSurface 3 "done"
return PP
end

function fiber(X::EllipticSurface)
b, pt, F = irreducible_fiber(X)
if b
Expand Down
43 changes: 19 additions & 24 deletions test/AlgebraicGeometry/Schemes/elliptic_surface.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
# The tests for elliptic surfaces have lead to random failure of the CI-tests,
# see issue no. 3676.
#
# They are now disabled, also because in general they tend to take quite long.
# We keep them, however, to allow for running them locally.
#=

@testset "elliptic surfaces" begin
@testset "trivial lattice" begin
k = GF(29)
Expand All @@ -27,9 +22,18 @@
X = elliptic_surface(E, 2)
triv = trivial_lattice(X)
@test det(triv[2])==-3
end

@testset "trivial lattice QQ" begin
Qt, t = polynomial_ring(QQ, :t)
Qtf = fraction_field(Qt)
E = elliptic_curve(Qtf, [0,0,0,0,t^5*(t-1)^2])
X3 = elliptic_surface(E, 2)
weierstrass_contraction(X3)
trivial_lattice(X3)
end

# This test takes about 1 minute
# This test takes about 30 seconds
@testset "mordel weil lattices" begin
k = GF(29,2)
# The generic fiber of the elliptic fibration
Expand All @@ -51,17 +55,9 @@
X1 = elliptic_surface(short_weierstrass_model(E)[1],2)
Oscar.isomorphism_from_generic_fibers(X,X1)
end
#=
# this test is quite expensive
# probably because it is over QQ
# ... and because there are loads of complicated singular fibers
Qt, t = polynomial_ring(QQ, :t)
Qtf = fraction_field(Qt)
E = elliptic_curve(Qtf, [0,0,0,0,t^5*(t-1)^2])
X3 = elliptic_surface(E, 2)
weierstrass_contraction(X3)
trivial_lattice(X3)
=#




@testset "elliptic parameter" begin
k = GF(29)
Expand Down Expand Up @@ -126,8 +122,7 @@ end


#=
# These tests are disabled, because they take too long, about 5 minutes. But one can run them if in doubt.
# most of the time is spent in decomposing the fibers
# These tests are disabled, because they are dependent on factorisation order...
@testset "two neighbor steps" begin
K = GF(7)
Kt, t = polynomial_ring(K, :t)
Expand Down Expand Up @@ -188,9 +183,9 @@ end
# The following should not take more than at most two minutes.
# But it broke the tests at some point leading to timeout,
# so we put it here to indicate regression.
for (i, g) in enumerate(values(gluings(default_covering(X))))
gluing_domains(g) # Trigger the computation once
end
#for (i, g) in enumerate(values(gluings(default_covering(X))))
# gluing_domains(g) # Trigger the computation once
#end

D_P = section(X, P)

Expand Down Expand Up @@ -272,4 +267,4 @@ end
P = Oscar.extract_mordell_weil_basis(torsion_translation)
@test length(P) == 2
end
=#

Loading