#9337: Add toric divisors
----------------------------------+-----------------------------------------
Reporter: vbraun | Owner: AlexGhitza
Type: enhancement | Status: needs_info
Priority: major | Milestone: sage-4.6
Component: algebraic geometry | Keywords:
Author: Volker Braun | Upstream: N/A
Reviewer: Andrey Novoseltsev | Merged:
Work_issues: |
----------------------------------+-----------------------------------------
Changes (by novoselt):
* status: needs_review => needs_info
Comment:
Some comments on "trac_9337_toric_divisors.patch" (I didn't go through all
of it yet).
Little ones:
1. Can we change printing from `The divisor x+y` to `Divisor x+y`? Such a
form seems to be more common in Sage (and saves 4 characters ;-))
1. It seems to me that the new module contains unnecessary imports (e.g.
`LatticePolytope`) and I think it is better to import only necessary
things so that it is clear what does this module depend on.
1. Line 138, description of OUTPUT in `ngens` seems to be left from some
other text.
1. The last part of example for `ToricVariety.divisor()` is confusing.
Can you add a comment in the documentation what exactly it should
demonstrate? Why there are two copies of the corresponding ray printed for
each divisor?
More global one:
1. I don't quite understand if `ToricDivisorGroup` is supposed to be a
group of T-Weil divisors only (as its documentation states) or of all
(Weil) divisors as it prints and somewhat works:
{{{
sage: P2 = toric_varieties.P2()
sage: P2.coordinate_ring().inject_variables()
Defining x, y, z
sage: G = P2.divisor_group()
sage: G
Group of ZZ-Divisors on 2-d CPR-Fano toric variety covered by 3 affine
patches
sage: print G(x)
The divisor x
sage: print G(x+y)
None
}}}
Is the last line supposed to raise an error (since `x+y` does not define
a T-divisor), or return some other divisor object of a different class
than `x`? The current behaviour certainly seems to be a bug to me.
I think that ideally there should be a separate group of T-Weil divisors
with divisor classes having lifts to this group. This group probably
should not derive from `DivisorGroup_generic`, but rather should have a
coercion map to it and both T-Weil and Weil groups should be accessible
from toric varieties. This way the divisor associated to "x" will have two
representation (one is likely to be more efficient and relevant for
computations) while "x+y" will have only a generic one. What do you think?
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9337#comment:15>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
