-
Notifications
You must be signed in to change notification settings - Fork 125
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
Elliptic surfaces fixes #4177
Elliptic surfaces fixes #4177
Changes from all commits
2dd38da
f4df3ad
493a6c9
6b5ffbd
4156b51
2f03447
fc4d29b
d6d4236
03df499
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -784,19 +784,6 @@ end | |
return C | ||
end | ||
|
||
function inherit_decomposition_info!(C::Covering, X::AbsCoveredScheme) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why remove this? Was it a duplicate? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. And it was wrong. Fortunately, it seemed to be used nowhere. |
||
D = default_covering(X) | ||
OOX = OO(X) | ||
if has_decomposition_info(D) | ||
for U in patches(C) | ||
V = __find_chart(U, D) | ||
phi = OOX(V, U) | ||
set_decomposition_info!(C, U, phi.(decomposition_info(D)[V])) | ||
end | ||
end | ||
return C | ||
end | ||
|
||
@attr Covering function trivializing_covering(M::HomSheaf) | ||
X = scheme(M) | ||
OOX = OO(X) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -345,7 +345,6 @@ end | |
end | ||
|
||
@attr Bool function has_dimension_leq_zero(I::Ideal) | ||
is_one(I) && return true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why remove this line? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's superfluous. It's usually not easier to compute this than the dimension. Both are the same Groebner basis computation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. Is the GB stored? Or could it be that it is discarded and just the |
||
return dim(I) <= 0 | ||
end | ||
|
||
|
@@ -354,14 +353,13 @@ 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 | ||
|
||
@attr Bool function has_dimension_leq_zero(I::MPolyQuoLocalizedIdeal) | ||
R = base_ring(I) | ||
P = base_ring(R)::MPolyRing | ||
J = ideal(P, lifted_numerator.(gens(I))) | ||
J = pre_saturated_ideal(pre_image_ideal(I)) | ||
has_dimension_leq_zero(J) && return true | ||
is_one(I) && return true | ||
return dim(I) <= 0 | ||
|
@@ -562,9 +560,12 @@ function extend!( | |
end | ||
|
||
function _iterative_saturation(I::Ideal, f::RingElem) | ||
fac = factor(f) | ||
return _iterative_saturation(I, typeof(f)[u for (u, _) in factor(f)]) | ||
end | ||
|
||
function _iterative_saturation(I::Ideal, f::Vector{T}) where{T<:RingElem} | ||
R = base_ring(I) | ||
for (u, k) in fac | ||
for u in f | ||
I = saturation(I, ideal(R, u)) | ||
end | ||
return I | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we turn this into a field of
EllipticSurface
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss this in #4183 .