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.

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

Two neighbor step in char 0 #4183

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
16 changes: 8 additions & 8 deletions experimental/Schemes/src/BlowupMorphism.jl
Original file line number Diff line number Diff line change
Expand Up @@ -644,18 +644,18 @@ function produce_object_on_affine_chart(I::StrictTransformIdealSheaf, U::AbsAffi
f_loc = covering_morphism(f)[U]
V = codomain(f_loc)
IE_loc = IE(U)
@assert isone(ngens(IE_loc)) "ideal sheaf of exceptional locus is not principal"
tot = pullback(f_loc)(J(V))
#return saturation_with_index(tot, IE_loc)
# It is usually better to pass to the simplified covering to do the computations
simp_cov = simplified_covering(X)
U_simp = first([V for V in patches(simp_cov) if original(V) === U])
a, b = identification_maps(U_simp)
# This used to be the following line. But we don't use the index, so we
# switch to the more performant version
# result, _ = saturation_with_index(pullback(a)(tot), pullback(a)(IE_loc))
result = _iterative_saturation(pullback(a)(tot), elem_type(OO(U_simp))[pullback(a)(u) for (u, _) in factor(lifted_numerator(first(gens(IE_loc))))])
return pullback(b)(result)
tot = pullback(f_loc)(J(V))
if isone(ngens(IE_loc))
result = _iterative_saturation(pullback(a)(tot), elem_type(OO(U_simp))[pullback(a)(u) for (u, _) in factor(lifted_numerator(first(gens(IE_loc))))])
return pullback(b)(result)
else
result, _ = saturation_with_index(pullback(a)(tot), pullback(a)(IE_loc))
return result
end
end

@attr Bool function is_prime(I::StrictTransformIdealSheaf)
Expand Down
71 changes: 61 additions & 10 deletions experimental/Schemes/src/elliptic_surface.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1220,6 +1220,17 @@ function _section(X::EllipticSurface, P::EllipticCurvePoint)
@vprint :EllipticSurface 3 "Computing a section from a point on the generic fiber\n"
weierstrass_contraction(X) # trigger required computations
PX = _section_on_weierstrass_ambient_space(X, P)

if !is_zero(P)
WC = weierstrass_chart_on_minimal_model(X)
U = original_chart(PX)
@assert OO(U) === ambient_coordinate_ring(WC)
PY = PrimeIdealSheafFromChart(X, WC, ideal(OO(WC), PX(U)))
set_attribute!(PY, :name, string("section: (",P[1]," : ",P[2]," : ",P[3],")"))
set_attribute!(PY, :_self_intersection, -euler_characteristic(X))
return WeilDivisor(PY, check=false)
end
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I switched to the creation of sections from points to come from the weierstrass chart directly. This has the advantage that the strict transforms do not have to be computed to realize this divisor on that chart. This computation was killing us in char. 0, so I hope this is ok.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Previously, it was a PrimeIdealSheafFromChart. But I deliberately changed that to StrictTransformIdealSheaf because it was more performant (and in principle it allows the algorithms to take advantage of the blowup structure).
Unless you can provide evidence that your change improves performance in a wide range of examples please leave it as is. Optionally you could include another dispatch with a type as first argument.

Copy link
Collaborator

Choose a reason for hiding this comment

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

One case where a PrimeIdealSheafFromChart will be faster is when we do only computations in the weierstrass chart e.g. for computing a linear system.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Exactly. The computations just didn't go through with the strict transform. Having a second argument as the return type should be OK, but we would still have to decide for a default.


for f in X.ambient_blowups
PX = strict_transform(f , PX)
end
Expand Down Expand Up @@ -1314,7 +1325,11 @@ function _prop217(E::EllipticCurve, P::EllipticCurvePoint, k)

# collect the equations as a matrix
cc = [[coeff(j, abi) for abi in ab] for j in eqns]
M = matrix(B, length(eqns), length(ab), reduce(vcat,cc, init=elem_type(base)[]))
mat = reduce(vcat,cc, init=elem_type(base)[])
@assert all(is_one(denominator(x)) for x in mat)
@assert all(is_constant(numerator(x)) for x in mat)
mat2 = [constant_coefficient(numerator(x)) for x in mat]
M = matrix(B, length(eqns), length(ab), mat2)
Comment on lines +1342 to +1343
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was making trouble over number fields:

ERROR: MethodError: no method matching (::AbsSimpleNumField)(::AbstractAlgebra.Generic.FracFieldElem{AbstractAlgebra.Generic.Poly{AbsSimpleNumFieldElem}})

Closest candidates are:
  (::AbsSimpleNumField)(::Singular.n_FieldElem{AbsSimpleNumFieldElem})
   @ Singular ~/.julia/packages/Singular/lsYSW/src/MessyHacks.jl:119
  (::AbsSimpleNumField)(::Vector{QQFieldElem})
   @ Hecke ~/.julia/packages/Hecke/7wI0D/src/NumField/Elem.jl:284
  (::AbsSimpleNumField)(::ZZRingElem)
   @ Nemo ~/.julia/packages/Nemo/tzyHK/src/antic/nf_elem.jl:1095
  ...

I guess the implicit conversion via B was not very clean, anyway, so I tried to clear this up. @simonbrandhorst : Could you have a look whether this is now reasonable?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Your change looks like it should work.

Alternatively in line 1302 it could be
R,ab = polynomial_ring(Bt,vcat([Symbol(:a,i) for i in 0:dega],[Symbol(:b,i) for i in 0:degb]),cached=false)
instead (base was replaced by Bt)
because we don't get any denominators anyway.

# @assert M == matrix(base, cc) # does not work if length(eqns)==0
K = kernel(M; side = :right)
kerdim = ncols(K)
Expand Down Expand Up @@ -1368,7 +1383,8 @@ function linear_system(X::EllipticSurface, P::EllipticCurvePoint, k::Int64)

I = saturated_ideal(defining_ideal(U))
IP = ideal([x*xd(t)-xn(t),y*yd(t)-yn(t)])
issubset(I, IP) || error("P does not define a point on the Weierstrasschart")
# Test is disabled for the moment; should eventually be put behind @check!
simonbrandhorst marked this conversation as resolved.
Show resolved Hide resolved
#issubset(I, IP) || error("P does not define a point on the Weierstrasschart")

@assert gcd(xn, xd)==1
@assert gcd(yn, yd)==1
Expand Down Expand Up @@ -1414,7 +1430,7 @@ function two_neighbor_step(X::EllipticSurface, F::Vector{QQFieldElem})
@assert scheme(parent(u)) === X
pr = weierstrass_contraction(X)
WX, _ = weierstrass_model(X)
# The following is a cheating version of the command u = pushforward(pr)(u)
# The following is a cheating version of the command u = pushforward(pr)(u) (the latter has now been deprecated!)
u = function_field(WX)(u[weierstrass_chart_on_minimal_model(X)])
@assert scheme(parent(u)) === weierstrass_model(X)[1]

Expand Down Expand Up @@ -1568,8 +1584,7 @@ function horizontal_decomposition(X::EllipticSurface, F::Vector{QQFieldElem})
@assert all(F4[i]>=0 for i in 1:length(basisNS))
D = D + sum(ZZ(F4[i])*basisNS[i] for i in 1:length(basisNS))
@assert D<=D1
l = Int(l)
return D1, D, P, l, c
return D1, D, P, Int(l), c
end

@doc raw"""
Expand Down Expand Up @@ -1662,6 +1677,15 @@ function reduction_to_pos_char(X::EllipticSurface, red_map::Map)
end::Tuple{<:Map, <:Map}
end

function reduction_of_algebraic_lattice(X::EllipticSurface)
return get_attribute!(X, :reduction_of_algebraic_lattice) do
@assert has_attribute(X, :reduction_to_pos_char) "no reduction morphism specified"
red_map, bc = get_attribute(X, :reduction_to_pos_char)
basis_ambient, _, _= algebraic_lattice(X)
return red_dict = IdDict{AbsWeilDivisor, AbsWeilDivisor}(D=>_reduce_as_prime_divisor(bc, D) for D in basis_ambient)
end::IdDict
end

@doc raw"""
basis_representation(X::EllipticSurface, D::WeilDivisor)

Expand All @@ -1677,11 +1701,12 @@ function basis_representation(X::EllipticSurface, D::WeilDivisor)
kk = base_ring(X)
if iszero(characteristic(kk)) && has_attribute(X, :reduction_to_pos_char)
red_map, bc = get_attribute(X, :reduction_to_pos_char)
for i in 1:n
@vprintln :EllipticSurface 4 "intersecting with $(i): $(basis_ambient[i])"

v[i] = intersect(base_change(red_map, basis_ambient[i]; scheme_base_change=bc),
base_change(red_map, D; scheme_base_change=bc))
red_dict = IdDict{AbsWeilDivisor, AbsWeilDivisor}(D=>_reduce_as_prime_divisor(bc, D) for D in basis_ambient)
HechtiDerLachs marked this conversation as resolved.
Show resolved Hide resolved
#red_dict_inv = IdDict{AbsWeilDivisor, AbsWeilDivisor}(D=>E for (E, D) in red_dict)
D_red = _reduce_as_prime_divisor(bc, D)
for (i, E) in enumerate(basis_ambient)
@vprintln :EllipticSurface 4 "intersecting in positive characteristic with $(i): $(basis_ambient[i])"
v[i] = intersect(red_dict[E], D_red)
end
else
for i in 1:n
Expand All @@ -1694,6 +1719,32 @@ function basis_representation(X::EllipticSurface, D::WeilDivisor)
return v*inv(G)
end

function _reduce_as_prime_divisor(bc::AbsCoveredSchemeMorphism, D::AbsWeilDivisor)
return WeilDivisor(domain(bc), coefficient_ring(D),
IdDict{AbsIdealSheaf, elem_type(coefficient_ring(D))}(
_reduce_as_prime_divisor(bc, I) => c for (I, c) in coefficient_dict(D)
)
)
end

function _reduce_as_prime_divisor(bc::AbsCoveredSchemeMorphism, I::AbsIdealSheaf)
result = pullback(bc, I)
has_attribute(I, :_self_intersection) && set_attribute!(result, :_self_intersection=>
(get_attribute(I, :_self_intersection)::Int))
return result
end

function _reduce_as_prime_divisor(bc::AbsCoveredSchemeMorphism, I::PrimeIdealSheafFromChart)
U = original_chart(I)
bc_cov = covering_morphism(bc)
V = __find_chart(U, codomain(bc_cov))
IV = I(V)
bc_loc = first(maps_with_given_codomain(bc_cov, V))
J = pullback(bc_loc)(IV)
set_attribute!(J, :is_prime=>true)
return PrimeIdealSheafFromChart(domain(bc), domain(bc_loc), J)
end
Comment on lines +1763 to +1772
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This method assumes that the prime ideal sheaf I stays prime when reduced via bc. This is not automatically the case, so we should probably warn the user to only use "sufficiently good reduction" here.


################################################################################
#
# patches for Oscar
Expand Down
3 changes: 3 additions & 0 deletions src/AlgebraicGeometry/Schemes/Sheaves/IdealSheaves.jl
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ end
end

@attr Bool function has_dimension_leq_zero(I::Ideal)
is_one(I) && return true
simonbrandhorst marked this conversation as resolved.
Show resolved Hide resolved
return dim(I) <= 0
end

Expand All @@ -353,6 +354,7 @@ end
P = base_ring(R)::MPolyRing
J = ideal(P, numerator.(gens(I)))
has_dimension_leq_zero(J) && return true
is_one(I) && return true
return dim(I) <= 0
end

Expand Down Expand Up @@ -1691,6 +1693,7 @@ function produce_object(
algorithm::Symbol=:pullback # Either :pushforward or :pullback
# This determines how to extend through the gluings.
)
U2 === original_chart(F) && return F.P
# Initialize some local variables
X = scheme(F)
OOX = OO(X)
Expand Down
Loading