Gorenstein and Picard index

[justus-springer]

Add functions gorenstein_index and picard_index for normal toric varieties. Also adds map_from_torusinvariant_cartier_divisors_to_class_group and map_from_picard_group_to_class_group.

[codecov]

Codecov Report

Merging #2667 (a480ba6) into master (47432f8) will decrease coverage by 0.20%.
Report is 2 commits behind head on master.
The diff coverage is 67.85%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2667      +/-   ##
==========================================
- Coverage   72.25%   72.05%   -0.20%     
==========================================
  Files         437      441       +4     
  Lines       61995    62339     +344     
==========================================
+ Hits        44796    44920     +124     
- Misses      17199    17419     +220     

Files Changed	Coverage Δ
.../ToricSchemes/src/NormalToricSchemes/Attributes.jl	48.90% <0.00%> (-3.04%)	⬇️
.../ToricVarieties/NormalToricVarieties/attributes.jl	93.08% <95.00%> (+0.08%)	⬆️

... and 12 files with indirect coverage changes

[HereAround]

Thank you for this great PR @justus-springer!

Generally speaking, your PR looks great. It adds good new code, documentation and valuable quick tests. Exactly what we aim for. Very nice!

I noticed a couple of things while checking your PR and left comments below.

[HereAround]

Similar to the above comment on the Gorenstein index - consider adding a few explaining words and/or a reference (if not introduced in CLS11 - I could not yet find it in there).

[justus-springer]

Unlike the Gorenstein Index, there are indeed not many references for this index (there is at least one though). Though it's very easy to define, since it's just the index of the Picard group in the class group, under the well-known embedding. I added it since it's so easy to compute and it relates to the Gorenstein index (the latter divides the former).

[HereAround]

Thank you explaining this index in the doc string. This is definitely sufficient to know what this index is.

I agree with @lkastner that it would be nice to have a reference and link it here. However, if you really cannot find anything, I think this should not be a leg breaker. Unless @lkastner objects, I am in this case good to go without reference.

[justus-springer]

I have added a reference in the docstring of picard_index, let me know what you think. It doesn't really go into detail about it, but it is a reference where the Picard index was used in a classification.

[justus-springer]

Thanks for the review @HereAround ! I replied to your comments and added a few more words explaining gorenstein_index and picard_index. If you think picard_index is too obscure (it does not occur in CLS11), I'd be fine with removing it from the PR.

[lkastner]

If you think picard_index is too obscure (it does not occur in CLS11), I'd be fine with removing it from the PR.

Could you maybe just add the reference? https://docs.oscar-system.org/dev/DeveloperDocumentation/documentation/#Updating-the-bibliography If you need this functionality it makes sense to have it in Oscar.

[HereAround]

@justus-springer For your information, the failures of the tests are (very very likely) not due to the changes in this PR. A fix is being worked on...

[HereAround]

@justus-springer Could you please rebase on the current master, so we can see if the tests complete? (There were significant changes this week.) Thank you!

[justus-springer]

Oh sorry, this doesn't look right. I tried rebasing, but I don't know what happened.
Edit: Should be fixed now.

[HereAround]

Thank you for making those changes @justus-springer . From my perspective, this PR is good to go. (Of course @lkastner can correct me, should I have overlooked something.)

One subtlety seems to be the Run tests / doctest (1.9, macOS-latest) (pull_request) , which times out after 150 minutes. I rerun the tests already, but to no avail. Maybe @benlorenz , @thofma or @fingolfin can propose a way forward?

[benlorenz]

One subtlety seems to be the Run tests / doctest (1.9, macOS-latest) (pull_request) , which times out after 150 minutes. I rerun the tests already, but to no avail. Maybe @benlorenz , @thofma or @fingolfin can propose a way forward?

#2685 will probably fix that timeout.

[HereAround]

One subtlety seems to be the Run tests / doctest (1.9, macOS-latest) (pull_request) , which times out after 150 minutes. I rerun the tests already, but to no avail. Maybe @benlorenz , @thofma or @fingolfin can propose a way forward?

#2685 will probably fix that timeout.

I understand that @lkastner is adding a bit more functionality to this other PR (hence, it is still a draft). So this might take a little while longer.