New method to convert toric divisors into Weil divisors #3076
Conversation
|
Can you post a testcase? In general: This means that the divisors we talked about with |
| b[j, 1] = ZZ(A[i, j]) | ||
| end | ||
| @show transpose(hb) | ||
| @show b |
There was a problem hiding this comment.
This is the part of the code I was talking about. The example I was running was:
IP = projective_space(NormalToricVariety, 2)
Oscar._weil_divisor_via_polymake(IP, elem_type(ZZ)[1, 0, 0])
|
@lkastner : Yes, thanks! That must have been the issue. Sorry for not posting the example code earlier. My connection was lost when I wrote my original messages. I will try again with your suggestions this afternoon. |
HechtiDerLachs
changed the title
|
Oops... Looks like we need to go through the sign convention once more. Tomorrow, then. |
| continue | ||
| function _torusinvariant_weil_divisors(X::NormalToricVariety; check::Bool=false, algorithm::Symbol=:via_polymake) | ||
| return get_attribute!(X, :_torusinvariant_weil_divisors) do | ||
| ray_list = rays(polyhedral_fan(X)) |
There was a problem hiding this comment.
I understand this function works as long as X is simplicial? So maybe:
| ray_list = rays(polyhedral_fan(X)) | |
| @req is_simplicial(X) "Variety must be simplicial" | |
| ray_list = rays(polyhedral_fan(X)) |
There was a problem hiding this comment.
I think @lkastner is the right person to ask. If this applies, we should add an @check.
| end | ||
|
|
||
| function _ideal_sheaf_via_polymake(X::NormalToricVariety, c::Vector{ZZRingElem}; check::Bool=false) | ||
| # Input: |
There was a problem hiding this comment.
Likewise here:
| # Input: | |
| @req is_simpicial(X) "Variety must be simplicial" | |
| # Input: |
| set_attribute!(td, :underlying_divisor=>result) | ||
| return result | ||
| function underlying_divisor(td::ToricDivisor; check::Bool=false, algorithm::Symbol=:direct) | ||
| return get_attribute!(td, :underlying_divisor) do |
There was a problem hiding this comment.
We could likewise have a check here for a simplicial toric variety, but this will be triggered by the other methods later down this method anyways, unless I am mistaken.
|
|
||
| @test K1 == K2 | ||
| @test K1 == K0 | ||
| @test K2 == K0 |
There was a problem hiding this comment.
Is this last line necessary? In theory, K1 == K2 and K0 == K1 implies K0 == K2. Maybe this is also true in OSCAR (for deeper reasons)?
There was a problem hiding this comment.
These trigger different calls for the comparison. So they test different code, indeed.
There was a problem hiding this comment.
Thank you for working on this @HechtiDerLachs . It seems that the doctests now complete (other than nightly), so I assume the sign error has been fixed. Great!
I left a few minor comments (nitpicks) above.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #3076 +/- ##
==========================================
+ Coverage 81.80% 81.81% +0.01%
==========================================
Files 556 556
Lines 74146 74317 +171
==========================================
+ Hits 60654 60805 +151
- Misses 13492 13512 +20
|
…#3076) [ToricSchemes] Extend conversion of toric divisors into (scheme theoretic) Weil divisors
@lkastner : After your explanations this morning I tried to implement what we talked about. However, when building the divisor locally, the MODULE_GENERATORS seem to not lay in the
weight_coneof the affine variety! Thensolve_mixedreturns empty results and I can't proceed.I left this as a work in progress. Could you take a look?