#9972: Add toric lattice morphisms
-------------------------------------+--------------------------------------
Reporter: novoselt | Owner: mhampton
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.6
Component: geometry | Keywords:
Author: Andrey Novoseltsev | Upstream: N/A
Reviewer: Volker Braun | Merged:
Work_issues: switch to FanMorphism |
-------------------------------------+--------------------------------------
Comment(by novoselt):
After some more contemplating and checking
{{{
$ grep "ambient_ray_indices()" sage/geometry/* |wc
30 179 2579
$ grep "ambient_ray_indices()" sage/schemes/generic/* |wc
13 85 1383
}}}
on Sage-4.6.alpha3, I think that you may be a bit overestimating the
danger. We have used this function in 43 places and so far everything is
working quite well. The "mistake" you ran into got quickly caught even
before the ticket got ready for a review. Also, ''why'' did you get this
mistake? Because you took a code from a place where the ambient definitely
was set correctly and moved it to the place where potentially other cones
maybe passed in. Finally, ''you'' actually have not done any mistake, you
just left a possibility for a user to make one by passing a wrong cone.
(In this case, I think, the mistake would be realized quite fast even if
the code did not throw up an exception.)
Adding a warning for calling `ambient_ray_indices` on the ambient itself
may not work because it may be called internally in some cycles. (I didn't
check but it is quite likely.) Besides, nothing is wrong with such a call.
I also strongly believe that this method must be open/documented/public,
because if we have used it in 43 places, it is quite important for
development and therefore users who build their own structures on top of
stuff shipped in Sage are likely to find it convenient in their code. I
personally use it all the time when working with varieties and sometimes
even regret that these indices are not the default output for cones, so
when I have a list of them I need to write a cycle calling this method.
A few months ago I already suggested switching to notation like
`fan.ray_indices(cone)` etc. but you opposed and pointed out that it is
not in the spirit of OOP. Now I actually completely agree and think that
it is very fortunate that we have not done so then. I even still see some
benefits of having special classes for cones of toric varieties and
morphisms (in particular, this problem would not occur ;-)) but for these
cases the disadvantages overweight.
Have I convinced you yet or should I abandon any hope?-)
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9972#comment:27>
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.
