#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):
Replying to [comment:34 vbraun]:
> 3.) is spectacularly ugly :-P
I wholeheartedly agree ;-)
> If you desperately want to keep `ambient_ray_indices()`, how about we
prefix any use in the toric varieties code with an assertion that makes it
explicit. This would be yet another way to make the implicit assumption
explicit and have it easily machine-verifiable.
I desperately want to keep it! I think that assertions are great - I don't
use them much mostly because I didn't know about them until reviewing some
of your code, but I am totally for them. However using them always before
`ambient_ray_indices()` is too harsh - I went through `cone.py, fan.py,
toric_variety.py, fano_toric_variety.py` and in all cases it was actually
already clear that `cone._ambient` is correct, e.g. because `cone` was
constructed in the same code. So I think that more generally we should use
assertions for input parameters, which was the problem in this case.
> On an unrelated note, I don't like `cone_of_fan = fan(cone)`, its too
similar to `Fan([cone])`. How about `Fan.cone_equivalent_to(cone)`, see
also the already-existing similar method `Fan.cone_containing(cone)`.
`fan(cone)` is similar to `Fan([cone])` only if your fan is called `fan`
;-) I was thinking of such format to mimic Element-Parent behaviour in
Sage, but:
* currently we don't use this model for cones and fans;
* if we did, then cones must be both elements (of fans) and parents (of
points), and this is not yet supported in Sage;
* I don't clearly see advantages of such a switch;
* I would like to have the same interface for "putting" cones into fans
and faces into cones.
So the attached patch implements for cones and fans methods `embed(cone)`.
`cone_equivalent_to` seems unclear to me despite of being long, but I am
not sure if `embed` is much better - what do you think of it? Maybe
`embed_cone/embed_face` would be more clear?
I propose a bit different implementation of this method than in your
patch. For fans it computes potential ray indices and then uses
`cone_containing` method. This should be faster than using
`cone_containing` directly on the rays and does not trigger cone lattice
computation. For cones it will go through all faces of the relevant
dimension, but instead of checking cone equivalence it still computes
potential ray indices and then looks for a face with them. It will not
work for non-strictly convex cones, where I also check for equivalence.
(Although it will not work yet anyway - we cannot compute face lattice in
such a case.) Documentation is mostly the same for fans and cones, but I
tried to explain and illustrate clearly what the function does and why one
should care.
I have also tweaked `Fan` constructor a little bit - now rays of the fan
generated by a singe cone will have the same order as this cone. Shuffling
bugged me some time before and again now that I was writing doctests for
embeddings. My general attitude to such situations is that users enter
data in the way they like, so it is good to preserve it as much as
possible.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9972#comment:35>
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.
