-
Notifications
You must be signed in to change notification settings - Fork 129
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
Durham preliminary cleanup #4345
base: master
Are you sure you want to change the base?
Durham preliminary cleanup #4345
Conversation
Add functionality for canonical divisor.
self intersection via adjunction
example erroxe
@@ -454,7 +454,31 @@ Return a cycle ``E`` equal to ``D`` but as a formal sum ``E = ∑ₖ aₖ ⋅ I | |||
where the `components` ``Iₖ`` of ``E`` are all sheaves of prime ideals. |
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.
Do we want to require that the I_k
are all distinct? I would say yes.
The new code assures this, but does not because we work with IdDict
s:
julia> P2 = projective_space(QQ,2);
julia> (s1,s2,s3) = homogeneous_coordinates(P2);
julia> J = ideal([s1]);
julia> J1 = ideal_sheaf(P2,J);
julia> J2 = ideal_sheaf(P2,J);
julia> D1 = algebraic_cycle(J1)
Effective algebraic cycle
on scheme over QQ covered with 3 patches
with coefficients in integer ring
given as the formal sum of
1 * sheaf of ideals
julia> D2 = algebraic_cycle(J2)
Effective algebraic cycle
on scheme over QQ covered with 3 patches
with coefficients in integer ring
given as the formal sum of
1 * sheaf of ideals
julia> D1 == D2
true
julia> irreducible_decomposition(D1+D2)
Effective algebraic cycle
on scheme over QQ covered with 3 patches
with coefficients in integer ring
given as the formal sum of
1 * sheaf of prime ideals
1 * sheaf of prime ideals
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.
Thanks for the catch. Indeed this is not the intended behaviour.
@@ -534,6 +558,25 @@ function integral(W::AbsAlgebraicCycle; check::Bool=true) | |||
return result | |||
end | |||
|
|||
# Getters for the components as honest algebraic cycles, not ideal sheaves. | |||
function components(::Type{T}, D::AbsAlgebraicCycle) where {T <: AbsAlgebraicCycle} |
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.
It seems to me the signature is lying a bit, since we do not get back an object of type T
.
But for usability reasons I can live with it. How do other parts of Oscar handle this?
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.
I think, this is the only feasible way. It should be legitimate to specify a non-concrete type as input for the first argument. Maybe we can add some type assertion?
# If this component was already seen in another patch, skip it. | ||
new_comp = PrimeIdealSheafFromChart(X, U, P) | ||
@vprint :Divisors 4 " $(any(new_comp == PP for PP in keys(ideal_dict)) ? "already found" : "new component")\n" | ||
any(new_comp == PP for PP in keys(ideal_dict)) && continue | ||
c = _colength_in_localization(num_ideal, P) | ||
@vprint :Divisors 4 " multiplicity $c\n" |
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.
It seems to me that we could use the decomposition info here?
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.
We can. I added a preliminary version of how to use this.
Given an `AbsWeilDivisor` `D` on a scheme `X`, create a principal divisor | ||
`div(f)` for some rational function, so that `D - div(f)` is supported | ||
away from the original support of `D`. |
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.
It seems to me that this may be impossible to achieve? |D|
may have a base locus (even with a divisorial part)
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.
I agree that "away" is a bit ambiguous here. What I had in mind were non-closed scheme-theoretic points which would then be different.
|
||
function is_zero(D::AbsAlgebraicCycle) | ||
all(is_zero(c) || is_one(I) for (I, c) in coefficient_dict(D)) && return true | ||
return all(is_zero(c) || is_one(I) for (I, c) in coefficient_dict(irreducible_decomposition(D))) && return true |
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.
Given my comment about the irreducible decomposition above this will lead to bugs.
But the fallback zero(D) == D
works already. So what is the benefit here?
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.
For some reason which I don't recall in detail, this did not work in an example we tried. Either way, this gives a shortcut which is potentially cheaper than comparing with the zero cycle.
else | ||
result = result + a*AlgebraicCycle(X, R, | ||
IdDict{AbsIdealSheaf, elem_type(R)}( | ||
[P + ideal_sheaf(E)=>one(R)]); check=false) |
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.
One because the computation is kind of lazy?
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.
No. One because it is really only the component we want to have. The coefficient a
is multiplied up front.
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.
What if the intersection is not transversal?
Co-authored-by: Simon Brandhorst <[email protected]>
Tests are failing, but I do not see why. Are any of these failures known? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4345 +/- ##
==========================================
- Coverage 84.52% 84.49% -0.03%
==========================================
Files 645 646 +1
Lines 85706 86027 +321
==========================================
+ Hits 72440 72691 +251
- Misses 13266 13336 +70
|
you are using deprecated functions |
@@ -455,32 +455,12 @@ where the `components` ``Iₖ`` of ``E`` are all sheaves of prime ideals. | |||
""" |
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.
Please adapt the docstring accordingly:
where the components
Iₖ
of E
are pairwise distinct sheaves of prime ideals.
Given an `AbsWeilDivisor` `D` on a scheme `X`, create a principal divisor | ||
`div(f)` for some rational function, so that `D - div(f)` is supported | ||
away from the original support of `D`. | ||
in (non-closed) scheme-theoretic points which are different from those of `D`. |
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.
I still do not understand. The quantors are missing. Are the supports disjoint? Or just not equal or ...?
This is a preliminary wrap-up of the things achieved in Durham. I would still like to keep the old branch #4314 since this is the one we started working on together and we would like to continue, eventually. In the meantime, I would like to get some changes in already so that this is not unnecessarily postponed.