#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):
It is not quite Python style to try to eliminate any possibility of user
mistakes by making sure that everything is called properly in proper
places. The assumption is that users know what they are doing ;-) These
methods assume that your cone is in some fixed fan:
* `adjacent`
* `ambient_ray_indices`
* `face_lattice` (indirectly - since elements will be cones of the same
fan)
* `faces` (indirectly)
* `facet_of`
* `facets` (indirectly)
* `star_generator_indices`
* `star_generators`
All of these methods are pretty important and convenient, in particular,
in many cases `ambient_ray_indices` is more useful than `rays` or
`ray_matrix` since it allows you to see clearly how different cones are
related to each other. It would be quite annoying if for using all these
methods it was necessary to mention fan explicitly.
Yes, there are bugs that appear because users make wrong assumptions, but
I don't think that it is a valid reason to require users to always
explicitly state all of their assumptions so that each function can check
that they are correct and when possible fix them.
In addition, as you pointed out in a sample code above, passing ambient
explicitly will mean that it always works correctly, but sometimes fast
and sometimes slow. This sometimes slow can be sometimes significantly
slow and the reason for keeping track of these ray indices and ambients is
precisely code optimization, otherwise all cones could store only their
rays and all fans - only their cones.
So I still think that if you have a cone and it is important that things
about this cone are computed relevant to a certain fan, you are
responsible for making sure that this cone "sits" in this fan and if not -
trying to convert it there. I see a point in making this conversion easy,
but not in making it mandatory every time you need it. Compare this
{{{
sage: indices = fan.ray_indices(cone)
sage: supercones = fan.cones_with_facet(cone)
}}}
and this
{{{
sage: indices = fan(cone).ambient_ray_incides()
sage: supercones = fan(cone).facet_of()
}}}
I think in the second case it is quite clear that
{{{
sage: cone = fan(cone)
sage: indices = cone.ambient_ray_incides()
sage: supercones = cone.facet_of()
}}}
is likely to be faster, plus it is a bit easier to read and there is only
one place where an exception can occur due to `cone` being incompatible
with `fan` at all.
I also think that using cones from a wrong fan is likely to be exposed
very quickly in actual work due to index-out-of range exceptions, so given
several month of working with current interface, I really don't want it to
change...
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9972#comment:23>
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.
