[sage-devel] Toric varieties - ready for review (please!)

14 views
Skip to first unread message

Andrey Novoseltsev

unread,
May 18, 2010, 6:17:20 AM5/18/10
to sage-devel
Dear developers,

I have just posted a series of patches implementing toric varieties in
Sage and will be very grateful if someone (or maybe even somemany!)
could help reviewing them. There are 3 patches in algebraic geometry
slightly modifying schemes modules, 3 small patches for lattice
polytopes (2 are already reviewed!), and 4 patches adding one module
each, so that the new additions are easier to review. Here they are:

Prerequisites:

#8675 - Remove AmbientSpace._constructor and fix consequences
#8682 - Improve AlgebraicScheme_subscheme.__init__ and
AmbientSpace._validate
#8694 - Improve schemes printing and LaTeXing

#8934 - Trivial bug in computing faces of non-full-dimensional lattice
polytopes (positively reviewed by Volker Braun)
#8936 - Expose facet inequalities for lattice polytopes
#8941 - _latex_ and origin for lattice polytopes (positively reviewed
by Volker Braun)

Main patches adding new modules:

#8986 - Add support for convex rational polyhedral cones
#8987 - Add support for rational polyhedral fans
#8988 - Add support for toric varieties
#8989 - Add support for Fano toric varieties

Everything was tested on sage.math using sage-4.4.2.rc0.

Potential reviewers may want to check /scratch/novoselt/sage-4.4.2.rc0-
toric on sage.math, which contains a fresh installation of Sage with
the above 10 patches applied. (I didn't do anything with access
rights, so I hope that anyone who has an account can use it.)

Also, compiled documentation is available at
http://sage.math.washington.edu/home/novoselt/en/reference/sage/geometry/cone.html
http://sage.math.washington.edu/home/novoselt/en/reference/sage/geometry/fan.html
http://sage.math.washington.edu/home/novoselt/en/reference/sage/schemes/generic/toric_variety.html
http://sage.math.washington.edu/home/novoselt/en/reference/sage/schemes/generic/fano_toric_variety.html

Hopefully, these patches can be merged soon, as while I was working on
these modules Volker Braun also was working on toric varieties, see
http://groups.google.com/group/sage-devel/browse_thread/thread/4100e7ced408f1e2#
The natural question is: should we merge his patches or mine? While I
am obviously biased, I think that we should first merge mine, which
provide basic framework, and then merge Volker's on top of mine with
necessary changes, since he was working more in the "mathematical"
direction rather than "frameworking."

Thank you!
Andrey

--
To post to this group, send an email to sage-...@googlegroups.com
To unsubscribe from this group, send an email to sage-devel+...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/sage-devel
URL: http://www.sagemath.org

Andrey Novoseltsev

unread,
May 18, 2010, 6:25:52 AM5/18/10
to sage-devel
To add to the previous post, my future plans are:

- Make sure that faces enumeration is the same everywhere, especially
for vertices and facets. This is relevant for lattice polytopes,
polyhedra, cones, and fans. We have had recently to fix some doctests
in Polyhedra because of random sorting and I have had to fix some of
the new doctests when I switched from 4.4 to 4.4.2.rc0.

- Write good plotting functions. For Fano toric varieties they must be
well-coordinated with lattice polytopes and I want to make the latter
ones more convenient and flexible.

- Improve morphisms and their doctests in schemes in general. My first
attack on documenting/improving schemes.generic.homset&morphism was
not very succesful, but I am going to continue. TestSuite(s).run()
should eventually work for schemes and ambient spaces! While it may
seem reasonable to first fix these issues and then add new classes, I
think that it is also good to have more "generic" ambient spaces for
testing purposes. Besides, with toric varieties merged I will
definitely understand at least some modules in schemes ;-)

- Implement classes to be used for lattices of cones/fans/lattice
polytopes.Ironically, lattice polytopes don't support lattices! For
cones and fans providing "lattice=" argument should "just work".

- While some of my "old" code got incorporated into the posted
patches, I still have quite a bit of written code which I hope to
convert into this framework.

- Quoting Volker: "implement everything that is known!"

Alex Ghitza

unread,
May 18, 2010, 7:09:10 AM5/18/10
to Andrey Novoseltsev, sage-devel
On Tue, 18 May 2010 03:17:20 -0700 (PDT), Andrey Novoseltsev <novo...@gmail.com> wrote:
> Dear developers,
>
> I have just posted a series of patches implementing toric varieties in
> Sage and will be very grateful if someone (or maybe even somemany!)
> could help reviewing them. There are 3 patches in algebraic geometry
> slightly modifying schemes modules, 3 small patches for lattice
> polytopes (2 are already reviewed!), and 4 patches adding one module
> each, so that the new additions are easier to review. Here they are:

Hi Andrey,

This is really great stuff! William pinged me a little while ago about
the schemes patches, and I said I would soon have some time to review
them. Well, I still don't have time :( but I'll try to do some of this
anyway.

In fact, I'll go and look at them right now.


Best,
Alex

--
Alex Ghitza -- http://aghitza.org/
Lecturer in Mathematics -- The University of Melbourne -- Australia

Volker Braun

unread,
May 18, 2010, 8:45:59 AM5/18/10
to sage-devel
Hi all,

I talked to David Cox and they are planning an appendix in their
upcoming book where they survey the different toric variety packages
in computer algebra systems. David was interested in covering Sage as
well; Their deadline is end of August.

Andrey's package is a nicer framework, but realistically can't compute
much. My package can compute mostly everything for which there is a
known toric algorithm, but doesn't have the nice framework.

To get anywhere before August, I would suggest the following:

- Add classes for N/M lattice to the framework.
- I'll merge my Cone/ToricVariety with Andrey's Cone_of_fan/
ToricVariety_field classes.
- Add the ChowCycle and Divisor classes from my package to compute
Chow homology and sheaf cohomology etc. Think about the best framework
later.

I'm mostly OK with the first three parts of Andrey's patches, but not
the FanoToricVariety (#8989) part:
- Can we rename it to PolytopalToricVariety or ReflexiveToricVariety
something like that? Most aren't Fano in the strict sense, so the name
grates a bit with me.
- Kahler cone and plotting should be implemented in the
ToricVariety_field base class, not here.

Finally a future project: To really deal with singular toric varieties
and/or Weyl/Q-Cartier divisors we need some way to find the integral
points in a non-integral polytope. LatticePolytope() can only deal
with integral polytopes (that is, spanned by integral vertices). There
is some mixed integer programming code out there that could be
assimilated.

Best wishes,
Volker

Dima Pasechnik

unread,
May 18, 2010, 10:35:02 AM5/18/10
to sage-devel
Barvinok's algorithm, as implemented in Latte (by J. de Loera and
others)
does such enumerations, so this seems to be a question of integrating
things properly...

Just in case,
Dima

Andrey Novoseltsev

unread,
May 18, 2010, 12:24:03 PM5/18/10
to sage-devel
I am confused with the google-group - I posted the second message last
night, but I don't see it. And it shows 5 message on the top, but I
can only see 4. I don't know if others can see it, but (it seems no,
so I'll copy it from a saved file:
---------------------------------------
To add to the previous post, my future plans are:

- Make sure that faces enumeration is the same everywhere, especially
for vertices and facets. This is relevant for lattice polytopes,
polyhedra, cones, and fans. We have had recently to fix some doctests
in Polyhedra because of random sorting and I have had to fix some of
the new doctests when I switched from 4.4 to 4.4.2.rc0.

- Write good plotting functions. For Fano toric varieties they must be
well-coordinated with lattice polytopes and I want to make the latter
ones more convenient and flexible.

- Improve morphisms and their doctests in schemes in general. My first
attack on documenting/improving schemes.generic.homset&morphism was
not very succesful, but I am going to continue. TestSuite(s).run()
should eventually work for schemes and ambient spaces! While it may
seem reasonable to first fix these issues and then add new classes, I
think that it is also good to have more "generic" ambient spaces for
testing purposes. Besides, with toric varieties merged I will
definitely understand at least some modules in schemes ;-)

- Implement classes to be used for lattices of cones/fans/lattice
polytopes. Ironically, lattice polytopes don't support lattices! For
cones and fans providing "lattice=" argument should "just work".

- While some of my "old" code got incorporated into the posted
patches, I still have quite a bit of written code which I hope to
convert into this framework.

- Quoting Volker: "implement everything that is known!"
---------------------------------------

Alex, thank you for such a quick response! Now we have 5 out of 6
prerequisites reviewed!

Volker, thank you for looking over the code and commenting it!

I will move the Kaehler cone to toric varieties.

Plot functions actually belong to fans, while toric varieties should
provide meaningful labels, although for Fano varieties it can make
sense to have something more special involving the polytope and its
unused points. In any case for now I would like to ask to keep the
existing function as is with the comment that it will be replaced soon
- it does provide a somewhat useful plot and it is probably not the
highest priority to get pretty pictures.

For "Fano"... I thought a lot about it as I also don't quite like that
it is necessary to keep in mind several more words. Proposition 3.5.5
in Cox&Katz book says "Delta is reflexive if and only if P_Delta is
Fano". That differs from the definitions in Nill's paper, who also
points out that sometimes people imply "smooth" when they say Fano.
How about this:
- leave the module name fano_toric_variety
- rename class to FanoToricVarietyCrepantResolution_field
- rename constructor to FanoToricVarietyCR
- say in the introduction that for us Fano means Gorenstein Fano
(omitting "Gorenstein" seems to be quite common, we can also consider
later allowing "non-canoninal Fano polytopes")
- change printing to "Crepant resolution of (Gorenstein?) Fano toric
variety of dimension n"
This way we also don't have to worry about coherency of subdivisions,
which is less trivial than crepancy.

Once this is settled, my first priority will be to add classes for
lattices. I don't think we should have separate classes for M and N,
because they are "the same" on the one hand and there may be need for
more than two (even in the same dimension) on the other one. I
envision "Lattice" as derived in Sage from ZZ^n with custom names,
dual(), and morphisms between them (which will automatically induce
toric morphisms between objects living in the same lattice - that way
we get automatic embeddings for all affine subvarieties including the
canonical embedding of the torus!). I will also go over you lattice
classes to see what else is needed. I should be able to finish this
and post a patch by the end of May (probably not for lattice polytopes
yet, as this will be quite involved and in fact I feel a need to
change a lot in lattice polytopes in general).

Dima, thank you for the reference!

Andrey

Volker Braun

unread,
May 18, 2010, 1:46:06 PM5/18/10
to sage-devel
Hi Andrey,

Why not allow arbitrary polytopal toric varieties instead of the
resolution of singular fano ones? The larger class still benefits from
the lattice polytope description and you can still talk about the
anticanonical divisor if you want.
FanoToricVarietyCrepantResolution_field is too long :-) Also, you
usually don't want to resolve all singularities.
GorensteinFanoToricVariety_field would work in a pinch.

The M/N lattices occur all the time and especially beginners tend to
confuse the two. This is why I find it very important to strictly
separate these two; In particular

1) All methods should raise errors whenever you pass a point from the
"wrong lattice".

2) _repr_() should make it unambiguous as to which lattice the point
lies in; I chose to print "N(1,2)".

I found it very useful to be able to construct them with N(1,2) etc,
this is implemented as a rather crude hack in my toric_lattices.py
file.

Volker

Andrey Novoseltsev

unread,
May 18, 2010, 11:03:29 PM5/18/10
to sage-devel
On May 18, 11:46 am, Volker Braun <vbraun.n...@gmail.com> wrote:
> Hi Andrey,
>
> Why not allow arbitrary polytopal toric varieties instead of the
> resolution of singular fano ones? The larger class still benefits from
> the lattice polytope description and you can still talk about the
> anticanonical divisor if you want.

Hi Volker,

It seems to me that usually the interest in mirror symmetry is in
varieties coming from refining reflexive polytopes. Batyrev-Borisov
constructions work with such varieties, construction of the
anticanonical hypersurface present in Fano module will not make sense
for a larger class of varieties, and this construction is definitely
not the culmination of this module - I hope to keep adding more stuff!
Since I personally almost always work with such varieties, I am more
likely to do correct things and less likely to create bugs if I write
code specifically for them. So if you think that it is necessary to
add a class to support arbitrary polytopal toric varieties, I still
would like to derive another one from it to work with fans coming from
reflexive polytopes. However, ToricVariety(FaceFan(Polytope)) already
allows one to create an arbitrary polytopal toric variety and
construct its partial subdivision, one just has to do a little more
work for as nice variable names as in FanoToricVariety(Polytope) and
has to give explicit rays rather then indices of points.

> FanoToricVarietyCrepantResolution_field is too long :-) Also, you
> usually don't want to resolve all singularities.
> GorensteinFanoToricVariety_field would work in a pinch.

If you looked over some modules in Sage guts, you could change your
threshold of "too long" ;-) However, you comment already shows that my
name suggestion was bad - I did NOT imply that it will be a complete/
maximal resolution! In fact, the purpose of the current
FanoToricVariety constructor is to allow flexible *partial*
subdivisions! So to clarify: are you OK to use
"GorensteinFanoToricVariety" for the constructor of toric varieties
coming from "partial crepant subdivisions of face fans of reflexive
polytopes," which are not really Fano any more? Putting all the words
into the name is definitely not a good idea, but we can add "PCR" in
the beginning or the end (corresponding to PartialCrepantResolution).
We can also put all the words explicitly into the _repr_ output or
even construct it based on the variety in question, so that initial
varieties don't mention resolutions and complete resolutions don't
mention partial.

>
> The M/N lattices occur all the time and especially beginners tend to
> confuse the two. This is why I find it very important to strictly
> separate these two; In particular
>
> 1) All methods should raise errors whenever you pass a point from the
> "wrong lattice".
>

Absolutely! a+b should work only if a and b are in the same lattice
and a*b only if they are in dual ones. But two lattices are not enough
for, say, Gross-Siebert constructions. I think they should be
distinguished similar to polynomial rings in Sage - if the name and
dimension are the same, there should be only one such a lattice
globally.

> 2) _repr_() should make it unambiguous as to which lattice the point
> lies in; I chose to print "N(1,2)".
>

Here I mostly disagree, I don't want to see the prefix, partially
because I want to allow arbitrary names for lattices like "N_sigma" or
"N_sigma748" - I'd rather not see such a long thing printed in front
of each point. However, this is very easy to control with some module
variable that users can set to switch between "N(1,2)" and just
"(1,2)". I am totally fine with having names "on" by default, as long
as I have a way to turn them "off". In fact, now I think that it would
be awesome to see these names as long as they are just M and N! How
about also latexing them with subscripts? Like "(1,2)_{N}"?

> I found it very useful to be able to construct them with N(1,2) etc,
> this is implemented as a rather crude hack in my toric_lattices.py
> file.

Perfect construction, I think the end result should be something like

sage: N = ToricLattice(2, "N")
sage: N(1,2)
N(1,2)
sage: toric_lattices.print_prefix =False
sage: N(1,2)
(1,2)

In addition, fans and cones can by default live in the lattice "N" of
the appropriate dimension. That way beginners don't actually have to
do that first step of creating a lattice, instead they can do

sage: fan = FaceFan(polytope)
sage: N = fan.lattice()
sage: M = N.dual()

One way or another they will have to define variables containing the
lattices, but that's pretty much unavoidable, I don't think it is a
good idea to put names like "N" and "M" into the global name space.

Thank you!
Andrey

Volker Braun

unread,
May 19, 2010, 4:17:31 AM5/19/10
to sage-devel
Hi Andrey,

I agree that the reflexive case is the most interesting, I just don't
understand where you need that in the FanoToricVariety code right now.
If we can relax restrictions without any cost then we should allow the
more general, right? On the other hand, if it would make the code more
complicated then screw it...

On May 19, 4:03 am, Andrey Novoseltsev <novos...@gmail.com> wrote:
> [...] we can add "PCR" in
> the beginning or the end (corresponding to PartialCrepantResolution).

Batyrev already defined the abbreviation

MPCP resolution =
(maximal projective crepant partial) resolution =
maximal subdivision of polyhedron

I think you want to do everything except the "maximal" (and perhaps
except the "projective", I don't see why you need to enforce that in
the Sage class), so how about PCPR or CPR? CPRFanoToricVariety_field?

> [...] How
> about also latexing them with subscripts? Like "(1,2)_{N}"?

Nice!

> One way or another they will have to define variables containing the
> lattices, but that's pretty much unavoidable, I don't think it is a
> good idea to put names like "N" and "M" into the global name space.

In fact, "N" is already in the global workspace :-)

sage: N(1/2)
0.500000000000000

Are you implementing the ToricLattice soon? Clearly this should be
done first...

Best wishes,
Volker

Volker Braun

unread,
May 19, 2010, 6:24:28 AM5/19/10
to sage-devel
Hi Andrey,

I've looked over your code and I think it would be good to extend the
class hierarchy of cones. I'm thinking about

Cone
Cone_of_fan(Cone)
keeps reference of fan
Cone_of_ToricVariety(Cone_of_fan)
keeps reference of toric variety
Cone_of_DomainOfToricMorphism(Cone_of_ToricVariety)
knows the image cone (smallest cone containing image)
Cone_of_RangeOfToricMorphism(Cone_of_ToricVariety)
knows the preimage cones

etc...

In order to allow this and future extensions, the
RationalPolyhedralFan() must create new cones from a factory(**) that
is passed to RationalPolyhedralFan.__init__(). The default factory
would create Cone_of_fan objects. Do you agree?

(**) One cannot use a virtual method of RationalPolyhedralFan,
ToricVariety(RationalPolyhedralFan) since the constructor always calls
base methods.

Apart from that, I think your framework is great to build upon. How
should we proceed? My suggestion would be
- Add ToricLattice framework
- Change RationalPolyhedralFan and its descendants to use (optional)
factories for new Cone objects.
- I'll review the patches
- Any additional functionality will then be implemented as separate
patches top of this patchset.

Best wishes,
Volker

Andrey Novoseltsev

unread,
May 19, 2010, 10:24:09 AM5/19/10
to sage-...@googlegroups.com
On Wed, May 19, 2010 at 2:17 AM, Volker Braun <vbrau...@gmail.com> wrote:
> Hi Andrey,
>
> I agree that the reflexive case is the most interesting, I just don't
> understand where you need that in the FanoToricVariety code right now.
> If we can relax restrictions without any cost then we should allow the
> more general, right? On the other hand, if it would make the code more
> complicated then screw it...
>

Hi Volker,

All resolutions are currently automatically crepant because reflexive
polytopes just don't have other points (except the origin). General
polytopes will require more care. Anticanonical hypersurface relies on
reflexivity for terminology, it is CY, and I don't think that the code
will produce the right divisor for non-reflexive polytopes. And, as I
said, the hope is to add more reflexive-specific code later. E.g.
generating function for Hodge numbers or direct use of combinatorial
formulas to compute Hodge numbers.

> On May 19, 4:03 am, Andrey Novoseltsev <novos...@gmail.com> wrote:
>> [...] we can add "PCR" in
>> the beginning or the end (corresponding to PartialCrepantResolution).
>
> Batyrev already defined the abbreviation
>
> MPCP resolution =
> (maximal projective crepant partial) resolution =
> maximal subdivision of polyhedron
>
> I think you want to do everything except the "maximal" (and perhaps
> except the "projective", I don't see why you need to enforce that in
> the Sage class), so how about PCPR or CPR? CPRFanoToricVariety_field?
>

CPR is perfect! I definitely don't want to have maximal and I'd rather
not have projective as it is harder to implement.

>> [...] How
>> about also latexing them with subscripts? Like "(1,2)_{N}"?
>
> Nice!
>
>> One way or another they will have to define variables containing the
>> lattices, but that's pretty much unavoidable, I don't think it is a
>> good idea to put names like "N" and "M" into the global name space.
>
> In fact, "N" is already in the global workspace :-)
>
> sage: N(1/2)
> 0.500000000000000
>

Wow... Well, at least it does something widely used ;-)
> Are you implementing the ToricLattice soon? Clearly this should be
> done first...

Yes, I just got this week very busy with other stuff. Hope to start on
the weekend.

Have to run now, will reply to your second message later (I even need
to read it first).

Thank you,
Andrey

Andrey Novoseltsev

unread,
May 19, 2010, 6:28:42 PM5/19/10
to sage-devel
On Wed, May 19, 2010 at 4:24 AM, Volker Braun <vbrau...@gmail.com> wrote:
> Hi Andrey,
>
> I've looked over your code and I think it would be good to extend the
> class hierarchy of cones. I'm thinking about
>
> Cone
> Cone_of_fan(Cone)
>  keeps reference of fan
> Cone_of_ToricVariety(Cone_of_fan)
>  keeps reference of toric variety
> Cone_of_DomainOfToricMorphism(Cone_of_ToricVariety)
>  knows the image cone (smallest cone containing image)
> Cone_of_RangeOfToricMorphism(Cone_of_ToricVariety)
>  knows the preimage cones
>
> etc...
>
> In order to allow this and future extensions, the
> RationalPolyhedralFan() must create new cones from a factory(**) that
> is passed to RationalPolyhedralFan.__init__(). The default factory
> would create Cone_of_fan objects. Do you agree?
>
> (**) One cannot use a virtual method of RationalPolyhedralFan,
> ToricVariety(RationalPolyhedralFan) since the constructor always calls
> base methods.


Hi Volker,

I do not support the idea of cones bound to toric varieties and
morphisms. Right now, for example, when you change the base field of a
toric variety it uses exactly the same fan and this is good not only
for speed and memory usage of this change itself, but also for cached
computations - working with any of the varieties different only by
fields will accumulate cone lattices, Gale transforms, etc. As far as
I know it would be very nice for certain applications to perform
computations over changing finite fields for the "same" toric variety.
If would be better if one did not have to reconstruct all fans and
cones each time, even if compatibility checks are suppressed.

On the other hand, I didn't think about it before but I definitely
agree with you that for morphisms we do need to store where do cones
go and where do they come from. What do you think about creating the
following:

- ToricLattice class.

- ToricLatticeMorphism, which is basically just a matrix and a pair of
lattices for domain/codomain (I prefer codomain rather than range
since this is what is used in Sage now, plus by "range" one can mean
the actual image of the domain which can be smaller than the
codomain). No relation to cones/fans yet.

- While mathematically it is pretty much the same, for implementation
purposes we should create something like FanMorphism (a single cone
can always be turned into a fan). It should be either derived from
ToricLatticeMorphism or just store one inside (second is probably
better). In addition it has fans living in the domain and the codomain
of the ToricLatticeMorphism. The constructor should check that these
fans are compatible with the lattice morphism (unless check=False).
This class should have methods like image_cone and preimage_cones.
These functions will return cones you have mentioned, given a cone or
its index in the appropriate fan (cones are hashable and can be used
as keys if we implement it as a dictionary). This will allow the same
fans to be shared by different morphisms.

Note that with the approach above these morphisms have nothing to do
with schemes and toric varieties and can be incorporated exclusively
into sage.geometry. This will make framework of cones/fans quite
complete, polished, and stable. Even inside the geometry modules it
will be nicely structured:
* ToricLattice is "independent"
* ToricLatticeMorphism depends on ToricLattice
* Cone depends (slightly) on ToricLattice
* Fan depends on Cone
* FanMorphism depends on Fan and ToricLatticeMorphism
The variant allowing construction of Fans with cones designed for
morphisms creates circular dependencies in this diagram (or even ties
it further to toric varieties!) and it will be easier to introduce
bugs. Another technical argument against passing cone constructors is
that it may interfere with pickling and therefore fail
TestSuite(...).run(). (There is already a method in Cones that
discards associated polyhedron during pickling because something funny
is going on there - it took me a few hours to figure out that this is
the problem, although I still don't know what happens in polyhedra).

Of course, in addition to all this talking I am also willing to
actually work and do this ;-)

FanMorphism should eventually induce morphisms between ToricVarieties
in the sense of general scheme morphisms in Sage (and I definitely
want to allow non-toric morphisms between toric varieties!).
Unfortunately, this part will likely require a lot of work on the
existing Sage morphisms. I am willing to do it as well but I very much
doubt that I can finish it by the end of August (surprisingly I got a
quite dense summer ahead ;-)). Fortunately, as I understand your
current code does not need it at all, as it relies on the
combinatorial description! So it should not be an obstacle for your
divisors/cohomology code for now and once "scheme morphisms of toric
varieties" are good enough, we can see if anything has to be "ported."

Having written the above arguments against deriving new classes and
recreating of objects I see that they may be applied also to my
classes ConeFace and Cone_of_fan. We may consider eliminating them,
but the main reason for their creation was to remember incidence
information in the face or cone lattice. They are also more likely to
be used interactively by users, so the ability to write
cone.fan_rays() instead of fan.ray_indices(cone) seems to be nice. As
I said, we may consider eliminating these classes if you agree with my
proposals above and think that they should be applied here as well.

Let me know what you think!
Andrey

Volker Braun

unread,
May 20, 2010, 4:37:53 AM5/20/10
to sage-devel
Hi Andrey,

I like the ToricLattice/ToricLatticeMorphism/FanMorphism, though I
would call the latter ToricMorphism. Domain/Codomain always reminds me
of "Cohomology Operations and Homology Cooperations" (Capter 17 in
Switzer) but if we want to use that nomenclature then thats fine with
me :-)

As for whether to subclass Cone/Cone_of_fan further, your proposal of
not doing this is precisely what I did the first time when I
implemented toric morphisms. Great Minds think alike :-) And I found
it to be a big mistake, and had to completely rewrite my code. Here
are some of the reasons:

* As you say, you end up with two totally different interfaces for the
same functionality: Everything that can be computed from a cone can be
gotten from
cone.compute_this(). And most things have a relative (fiberwise)
version that you then get from morphism.compute_this(cone)? That gets
very confusing after a while.

* Cones of a fan don't just need to know their relative positions,
you'll also want choices of lattice generators and lattice quotients
cached by the cone. For all of this there is a relative version. If
you don't allow one to extend the Cone to a Cone_of_fan and a
Cone_of_domain then you need to cache everything in hashes
{cone:property} that you maintain by hand. I would argue that
subclassing cones further decreases the complexity (and probability of
bugs), not the other way round.

* OOP prefers cone.compute_this() over fan.compute_this(cone).

* There are no circular dependencies / imports.

* I've never worked with toric varieties over finite fields, but I'm
pretty sure that change of base field does not change the fan or
anything in the combinatorial description. After all, the M-lattice is
just a fancy way of enumerating monomials and there are still
infinitely many polynomials over finite fields.

Finally, I don't understand your claim that this would be a problem
for pickling. Can you elaborate on that? There are circular
references, but that is handled just fine by cpickle?

Best wishes,
Volker

Andrey Novoseltsev

unread,
May 20, 2010, 10:30:57 AM5/20/10
to sage-devel
Hi Volker,

That definitely makes sense, let me think a bit about it and then post
the comments/agreements.

You cannot pickle functions, so you cannot pass a factory to construct
cones (because you may need to have more cones later, so you need to
store it). If you apply my cone patch and remove __reduce__ method or
some other __ __ that gets rid of polyhedron, you will see some error
messages from TestSuite or if you try to dump a cone explicitly. Those
messages are not illuminating, but I found out that dropping
polyhedron works. It may be good to actually track the problem
further. However, passing and storing a class should work.

Thank you,
Andrey

Jason Grout

unread,
May 20, 2010, 1:31:01 PM5/20/10
to sage-...@googlegroups.com
On 5/20/10 9:30 AM, Andrey Novoseltsev wrote:


> You cannot pickle functions


According to

http://docs.python.org/library/pickle.html#what-can-be-pickled-and-unpickled

you can pickle certain types of functions. I don't know if that helps
at all, but your statement about pickling functions intrigued me.

Thanks,

Jason

Volker Braun

unread,
May 20, 2010, 3:13:43 PM5/20/10
to sage-devel
To summarize: Plain python cannot pickle inner classes or functions:

>>> class outer(object):
... class inner(object):
... pass
>>> o = outer()
>>> pickle.dumps(o)
(works)
>>> i = o.inner()
>>> pickle.dumps(i)
(ERROR)

The work-around is to make the cone factory a top-level class (or
function), then it can be pickled just fine.

Volker

Andrey Novoseltsev

unread,
May 20, 2010, 11:52:57 PM5/20/10
to sage-devel
Hi Volker,

Fine, I am now convinced that you are right - we do need more
subclasses of cones ;-) And probably the same for fans?

Since I am still a bit paranoid about speed and memory consumption,
what do you think of the following scheme:

Make classes EnhancedFan and Enhanced Cone (not exposed to the end
user). Their use will be like this (as well as for derived classes):

sage: econe = EnhancedCone(usual_cone)

which will return the enhanced cone associated to usual_cone, and

sage: efan = EnhancedFan(usual_fan, EnhancedCone)

which will return the enhanced fan with enhanced cones (and it can
take any derived class as an argument).

I think that ideal implementation for these classes would be to NOT
derive from Fan and Cone, but instead store an object of the
corresponding class in an internal attribute and send all unknown
methods to these objects via __getattr__. This will not only eliminate
copying of ray structures, but will also allow sharing of all cached
data computed after creation of these enhanced objects. If there are
any issues - switching to honest derivation will not change the
behavior for classes derived from these, where you can do whatever you
want to accommodate morphisms but still have direct access to rays(),
fan_rays(), and all other things available in cones.

Since it seems that we are close to agreeing on the above or something
quite similar, we can proceed like this:

Me (soon):
- add ToricLattice and ToricLatticeMorphism classes
- make cones and fans use these lattices by default
- change fans to allow easy extensions
- change names in fano_toric_variety and rewrite the introduction
accordingly

You:
- review these patches
- add fan morphisms (in a "combinatorial way" to sage.geometry)
- add other computations

Me:
- review your patches
- eventually make scheme-type morphisms of toric varieties work nicely
(in sage.schemes)

Thank you,
Andrey

Volker Braun

unread,
May 21, 2010, 7:24:22 AM5/21/10
to sage-devel
Hi Andrey,

Just to clarify, I only wanted to subclass Cones, but not Fans. The
fan is a container class for cones and that doesn't change. In
particular, I never felt the need to add any functionality at that
level in my code.

As for sharing of cached data, I think a clearer way would be to
define a copy method in addition to the creation in the factory. Then
we just have to extend RationalPolyhedralFan to
* create new cone if cone was specified by point indices, and
* copy if the cone is already a cone of a (different) fan (that is,
c.is_Cone()==True)

For example:

class Cone_of_fan_Factory(SageObject):
def new(self, fan_rays, fan_generating_cones, fan):
return Cone_of_fan(fan_rays, fan_generating_cones, fan)
def copy(self, cone):
new_cone = copy(cone)
cone.__class__ = Cone_of_fan

I think your timeline looks good!

Best wishes,
Volker

Andrey Novoseltsev

unread,
May 21, 2010, 12:13:56 PM5/21/10
to sage-devel
> For more options, visit this group athttp://groups.google.com/group/sage-devel
> URL:http://www.sagemath.org


Hi Volker,

I imagine creating a toric morphism between fans in such a way:

sage: fan1 = ...
sage: fan2 = ...
sage: data = ...
sage: phi = ToricMorphism(phi, fan1, fan2)
sage: other_data
sage: psi = ToricMorphism(other_data, fan2, fan1)
...
(even more morphisms involving these and other fans in different
combinations)

Where fan1 and fan2 are usual standalone fans (e.g. face fans of
polytopes) and data and other_data are either a matrices or
ToricLatticeMorphism objects (this class is basically a wrapper around
a matrix, but I want to create it to allow extra checks and
conversions for "compatibility of lattices" in the spirit of having
lattices everywhere else). The constructor of ToricMorphism will then
create two "copies" of each of fan1 and fan2 with cones designed for
domain and codomain. The natural way to do it seems to be like I wrote
before:

sage: domain = EnhancedFan(fan1, Cone_of_domain)
sage: codomain = EnhancedFan(fan2, Cone_of_codomain)

This "creation of fans from fans" does not seem to be natural from the
point of view of the Fan constructor, so it seems reasonable to have a
separate function or class to do it, even if there is no need for
extra methods. Plus, it will allow me to hide (from you ;-)) the
actual implementation of this. Copying suggested by you will work and
will share cached data, but, as I understand, only those that were
created *before* copying. Storing a reference to the original fans and
cones will allow sharing of all the future "general" cached
information as well, phi and psi will store only data which are
related to the actual morhphisms, which clearly must be separate.
Again, from your perspective in morphisms module working with these
"enhanced" fans and cones should be in any way different than direct
use of RationalPolyhedralFan and deriving from Cone_of_fan - same
methods will be available, giving the same results (except that
sometimes these results should be available faster). In fact, I am
considering doing it "the easy way" for now and worrying about
optimization later, but inserting these classes now will allow this
future optimization without disrupting the morphism module in any way.

I especially worry about cone/fan computations that trigger calls to
associated lattice polytopes and polyhedra (cone_lattice is a good
example) - if these calls result in invoking PALP or cdd, each
operation is at least about 0.1s long, no matter how simple is the
computation. With 10 cones it will be noticeable, with 100 - annoying.
There is a workaround for this in lattice polytopes allowing computing
data for many polytopes in a single run of PALP, but it is not always
possible to use it.

Some other comments:

I think that cone.is_Cone() is a bad testing syntax since "not-cones"
are likely not to have such a method at all. Instead, it seems that
many modules in Sage define functions like is_Cone(cone), which can
then accept absolutely any argument and return True or False without
raising exceptions. I think I have written such functions for
everything but fans, which probably should be added too.

Did you do any work on your module for toric lattices since the
version posted here: http://www.stp.dias.ie/~vbraun/Sage/ ? If yes,
could you please update it? It will be good for me to see what things
do you need from them, in case there is something that I would not
think about.

I saw that you have some interface to get triangulations from TOPCOM.
Can you perhaps make an experimental package for it? I tried to
install it sometime ago but was not successful as there were some
build errors and my Linux knowledge is not that great. My code does
include an algorithm to produce some subdivision, but I imagine that
TOPCOM is faster and has way more features. Making it a standard
package is a bit involved and will require long-term commitment from
you, but since it is a relatively simple thing, I think that an
experimental one will work just fine and then there can be more
subdividing options for fans. (I already tried to write it with
several algorithms in mind.)

I also really want to have that "database of named toric varieties"
implemented in your files, although I'd rather have a separate
function (or even module) for this functionality instead of building
it into ToricVariety(...). So please do include it!

Thank you,
Andrey

Volker Braun

unread,
May 21, 2010, 1:24:43 PM5/21/10
to sage-devel
Ok, I agree that it would be nice for all derived cones to share the
base cached data. How about Cone() just stores all of its cached data
in a hash self.shared_cache (or so) instead of in self. For the
computer thats essentially the same as what you are suggesting, but in
the implementation you wouldn't need to fiddle with __getattr__ and
would keep an inheritance chain like

Cone -> Cone_of_fan Cone_of_toric_variety -> Cone_of_codomain

Even if the implementation details are not important to the user, I
think it would help with the future maintenance.

On May 21, 5:13 pm, Andrey Novoseltsev <novos...@gmail.com> wrote:
> I especially worry about cone/fan computations that trigger calls to
> associated lattice polytopes and polyhedra

In the long run we should write a cython module that does the basic
cone/polyhedron operations instead of parsing palp or cdd output. That
said, I've looked at the palp source code and I'm certain only Max
understands what it does. I think writing something new based on the
Parma Polyhedral Library (C++) is the way to go.

> Some other comments:
> define functions like is_Cone(cone)

Yes, definitely nice syntactic sugar.

> Did you do any work on your module for toric lattices since the
> version posted here:http://www.stp.dias.ie/~vbraun/Sage/?

No, I haven't made any changes.

> I saw that you have some interface to get triangulations from TOPCOM.

The main problem right now is that TOPCOM segfaults when checking
regularity (=coherence). But I think that I can fix that quickly. The
interface I have written works for everything else, though. You
probably shouldn't spend much time implementing some subdivision
algorithm in python.

Volker

--
To post to this group, send an email to sage-...@googlegroups.com
To unsubscribe from this group, send an email to sage-devel+...@googlegroups.com

Andrey Novoseltsev

unread,
May 21, 2010, 1:48:59 PM5/21/10
to sage-devel
Hi Volker,

On May 21, 11:24 am, Volker Braun <vbraun.n...@gmail.com> wrote:
> Ok, I agree that it would be nice for all derived cones to share the
> base cached data. How about Cone() just stores all of its cached data
> in a hash self.shared_cache (or so) instead of in self. For the
> computer thats essentially the same as what you are suggesting, but in
> the implementation you wouldn't need to fiddle with __getattr__ and
> would keep an inheritance chain like
>
> Cone -> Cone_of_fan Cone_of_toric_variety -> Cone_of_codomain
>
> Even if the implementation details are not important to the user, I
> think it would help with the future maintenance.

Good point, that would be cleaner and easier!

>
> On May 21, 5:13 pm, Andrey Novoseltsev <novos...@gmail.com> wrote:
>
> > I especially worry about cone/fan computations that trigger calls to
> > associated lattice polytopes and polyhedra
>
> In the long run we should write a cython module that does the basic
> cone/polyhedron operations instead of parsing palp or cdd output. That
> said, I've looked at the palp source code and I'm certain only Max
> understands what it does. I think writing something new based on the
> Parma Polyhedral Library (C++) is the way to go.
>

I also tried to read PALP code a couple times but gave up. Avoiding
system call will definitely make things better from any point of view,
but that's probably quite a bit of work. I think it still would be
good to keep PALP in Sage since it is quite fast and it is the only
program for computing nef partitions I am aware of. Plus - it is
already in and working.

> > I saw that you have some interface to get triangulations from TOPCOM.
>
> The main problem right now is that TOPCOM segfaults when checking
> regularity (=coherence). But I think that I can fix that quickly. The
> interface I have written works for everything else, though. You
> probably shouldn't spend much time implementing some subdivision
> algorithm in python.

I already have some subdivision implemented and it is used by default,
so no extra work is needed at this point. I am absolutely sure that we
should keep it unless we write something else independent of packages
not included into Sage (in which case we still can keep it as an
alternative). The reason is that no matter how inefficient it is - it
will work in the standard Sage installation (and it is actually
sufficiently fast to be useful). If you will make TOPCOM a standard
package, that would be great, but as I understand it is not that easy
to have new packages accepted as standard in Sage.

Thank you,
Andrey

Andrey Novoseltsev

unread,
May 27, 2010, 1:08:26 AM5/27/10
to sage-devel
Hi Volker,

The first version (already documented and doctested) of toric lattices
is on trac:
http://trac.sagemath.org/sage_trac/ticket/9062
so you can take a look and comment if something is very bad. It is not
quite ready for review yet, but I don't anticipate changing existing
code - only adding some more.

These lattices allow creation and printing as "N(1,2,3)" and do not
allow mixing of wrong lattices for a given operation, as you wanted.
They behave "like ZZ^n", as I wanted. Elements are implemented in
Cython and as I was working on them, I was trying to make sure that
there are no big performance hits compared to just integer vectors. I
still have to work on matrix operations and support for morphisms, but
I don't anticipate any problems (everything is already in the Sage
structure files, I just need to make sure that all the types are in
agreement in necessary places). Then I will change cones and fans to
use these lattices by default instead of ZZ^n.

I wanted to have an option of suppressing lattice names in elements
output, but so far I have found your idea of having them extremely
convenient for debugging ;-) So instead of writing functions to get
rid of the names now, I put it on my todo list for the future since
there are more urgent things to do.

Thank you,
Andrey

Andrey Novoseltsev

unread,
May 27, 2010, 10:53:37 PM5/27/10
to sage-devel
Hi Volker,

I put a new version of toric lattices patch. Now it should work nicely
with plain vectors and matrices, in particular homomorphisms seem to
do what they should (there is a simple example in the end of "module
level" documentation). I don't think anymore that there is any need
for a separate class ToricLatticeMorphism since standard morphisms
between free modules seem to work just fine. I had to prohibit
expressions like M(N(1,2,3)), but I think it is reasonable - if one
wants to convert elements between lattices, there is a possibility to
make an explicit morphism without any restrictions and I have shown
how to do the above conversion in the documentation. By the way - note
that a morphism given by a matrix M will act on a vector v like v*M,
that's how it is done in Sage.

It is quite possible that there are some situations that still have to
be addressed/resolved, but I propose to use this patch as an initial
version and apply fixes/modifications on top of it and other relevant
modules. I will now work on changing the default lattice in cones and
fans. That should be easy, but will probably involve a lot of doctest
fixing because of lattice names printed with rays.

Thank you,
Andrey
Reply all
Reply to author
Forward
0 new messages