#8987: Add support for rational polyhedral fans
----------------------------------+-----------------------------------------
Reporter: novoselt | Owner: mhampton
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-4.4.4
Component: geometry | Keywords:
Author: Andrey Novoseltsev | Upstream: N/A
Reviewer: Volker Braun | Merged:
Work_issues: |
----------------------------------+-----------------------------------------
Comment(by vbraun):
1) I agree that we should test `==` without identifying `GL(n,Z)` images
as it is expensive. But I'm afraid of the following, which is probably a
common use case:
{{{
sage: diamond = lattice_polytope.octahedron(2)
sage: P1xP1 = FaceFan(diamond)
sage: Cone([(0,1), (1,0)]) in P1xP1(dim=2)
False
sage: Cone([(1,0), (0,1)]) in P1xP1(dim=2)
True
}}}
Also, note that `Fan()` is implicitly sorting:
{{{
sage: fan1 = Fan([Cone([(1,0), (0,1)])])
sage: fan2 = Fan([Cone([(0,1), (1,0)])])
sage: fan1==fan2
True
}}}
I would argue that is is ok to treat fans and cones a bit different, one
will compare cones often but fans only seldomly. I'm happy with comparison
of fans to depend on the order of the rays and generating cones, otherwise
all derived quantities (like cohomology generators of toric varieties)
will not be equal (only isomorphic). But for `cone1==cone2` to depend on
the order of the rays is neither helpful nor intuitive.
2) `Fan()` should always error out if the cones are not generating cones.
In particular, this one from the documentation
{{{
sage: cone1 = Cone([(1,0), (0,1)])
sage: cone2 = Cone([(0,1), (-1,-1)])
sage: cone3 = Cone([(-1,-1), (1,0)])
sage: P2 = Fan([cone1, cone2, cone2])
sage: P2.ngenerating_cones()
2
}}}
should have raised an exception.
In normal use you'll never want to type in all the cones of the fan since
there are so many. But its easy to get confused and add a cone that turns
out to be not a generating cone, and its nice to catch this when
generating the `Fan`.
There is certainly a need for a function that extracts the generating
cones from a collection of cones, but I don't think it should be implicit
in the Fan constructor.
3) Rename `Cone_of_fan.fan_generating_cones()` to `star_generators()`.
There are similar methods to this one that we probably want to add later,
so let me just throw out some names:
* `Cone_of_fan.faces()`: all subcones of the cone
* `Cone_of_fan.facets()`: subcones of one dimension lower
* `Cone_of_fan.bounds()`: supercones of one dimension higher
* `Cone_of_fan.star()`: the star of the cone
* `Cone_of_fan.adjacent()`: Adjacent cones (of the same dimension)
4) Do we need to know the set of ray indices, that is
`set(cone.fan_rays())`, of a cone often? Right now there is the function
`cone_to_rays(cone_index)` that is only used as a helper in
`cone_lattice()`. If it is just a helper function, then it should be
`_cone_to_rays()`. If you want to expose that functionality, it should be
stored in the `Cone_of_fan` and retrieved via `cone.fan_rays_set()` or so.
I also think that `cone.rays_idx()` would be a better name than
`cone.fan_rays()`, but if you disagree then I can live with the current
name as well :-)
5) similarly to 4), `ray_to_cones()` is not very self-explanatory. We
obviously need a way to find out which cones contain a given set of rays.
I would prefer one (or all) of the following
* `ray_to_cone(ray_index)`: the 1-cone spanned by the ray
* `smallest_cone_containing(*Nlist)`: the smallest cone containing all
points (which can be specified by a ray index or as a N-lattice element).
The functionality of `ray_to_cones` would then be provided by
`ray_to_cone(i).star_generators()`.
Let me know what you think...
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/8987#comment:9>
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.
