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

[ToricVarieties] Bugfix in ideal of linear relations #2472

Merged
merged 1 commit into from Jun 15, 2023

Conversation

HereAround
Copy link
Member

@HereAround HereAround commented Jun 14, 2023

As reported by Giosuè Muratore on slack, an intersection number is computed incorrectly for a particular smooth, Fano 3-fold:

e1 = [1,0,0]
e2 = [0,1,0]
e3 = [0,0,1]
m = 2
ray_generators = [e1, -e1, e2, e3, - e2 - e3 - m * e1]
max_cones = [[1,3,4], [1,3,5], [1,4,5], [2,3,4], [2,3,5], [2,4,5]]
X = normal_toric_variety(ray_generators, max_cones; non_redundant = true)
integrate(cohomology_class(anticanonical_divisor(X))^3)

The answer should be 62 and not 104 (https://www.fanography.info/2-36). Note that the following line gives the desired answer of 62:

integrate(cohomology_class(anticanonical_divisor_class(X))^3)

I investigated this and found the following:

  • Both commands compute linearly equivalent toric divisors.
  • They then map these divisors to distinct elements of the cohomology ring.
  • The "sanity check" to prevent this from happening is the ideal of linear relations. Namely, the cohomology ring is obtained as quotient of the Cox ring by this ideal. This should ensure that linearly equivalent divisors are mapped to the same element of the cohomology ring.
  • It was thus natural to look into the ideal of linear relations, and sure enough I found a bug in that routine. Specifically, we must use primitive elements u_i rather than the ray generators rho_i (cf. CLS equ. 12.4.3 on page 593).
    * The fix in this PR uses a different route. Under the assumption that the Cox ring is graded by Z^n (that is at least covering all my applications), we can think of this grading as a matrix over Z. Then the generators of the ideal of linear relations are 1:1 to a basis of the kernel of said matrix.

I have modified the code accordingly. Thereby, the above example works fine. As refined check for the future, I have added this example to the documentation of the command integrate for cohomology classes.

cc @lkastner

@HereAround HereAround marked this pull request as draft June 14, 2023 21:59
@HereAround HereAround requested a review from lkastner June 15, 2023 08:26
@HereAround HereAround marked this pull request as ready for review June 15, 2023 08:41
@codecov
Copy link

codecov bot commented Jun 15, 2023

Codecov Report

Merging #2472 (cacc66b) into master (98f7ba1) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head cacc66b differs from pull request most recent head 3dd7045. Consider uploading reports for the commit 3dd7045 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2472      +/-   ##
==========================================
+ Coverage   73.12%   73.13%   +0.01%     
==========================================
  Files         400      400              
  Lines       53590    53591       +1     
==========================================
+ Hits        39187    39196       +9     
+ Misses      14403    14395       -8     
Impacted Files Coverage Δ
...try/ToricVarieties/AlgebraicCycles/constructors.jl 56.66% <ø> (ø)
...ricVarieties/AlgebraicCycles/special_attributes.jl 81.48% <ø> (ø)
...ometry/ToricVarieties/CohomologyClasses/methods.jl 60.00% <ø> (ø)
.../ToricVarieties/NormalToricVarieties/attributes.jl 92.85% <100.00%> (+0.03%) ⬆️

... and 4 files with indirect coverage changes

@HereAround HereAround force-pushed the Bugfix branch 2 times, most recently from 171391d to d170a22 Compare June 15, 2023 10:17
@lkastner lkastner enabled auto-merge June 15, 2023 12:04
@lkastner lkastner merged commit 4b5288a into oscar-system:master Jun 15, 2023
9 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants