Opened 10 years ago
Closed 9 years ago
#12892 closed enhancement (fixed)
Toric fibration morphisms
Reported by:  Volker Braun  Owned by:  Alex Ghitza 

Priority:  major  Milestone:  sage6.2 
Component:  algebraic geometry  Keywords:  sd40.5 sd53 toric 
Cc:  Andrey Novoseltsev, Jan Keitel  Merged in:  
Authors:  Volker Braun  Reviewers:  Andrey Novoseltsev 
Report Upstream:  N/A  Work issues:  
Branch:  b3f06ed (Commits, GitHub, GitLab)  Commit:  b3f06ed5bb902f1f4ce0f3f11935cf62101137d0 
Dependencies:  #12361, #13023, #14353  Stopgaps: 
Description (last modified by )
This ticket provides more morphisms that are associated to toric varieties:
 embedding of an orbit closure
 embedding of a fiber of a toric morphism
 pullback of divisors
Use the git branch!
Attachments (4)
Change History (65)
comment:1 Changed 10 years ago by
Authors:  → Volker Braun 

Cc:  Andrey Novoseltsev added 
Description:  modified (diff) 
Status:  new → needs_review 
comment:2 Changed 10 years ago by
Reviewers:  → Andrey Novoseltsev 

comment:4 Changed 10 years ago by
Dependencies:  → #12361 

comment:5 Changed 10 years ago by
I find the first patch a bit difficult to understand due to mixing several things defining the embedding: a cone and two dictionaries of rays. Also defining_cone
argument is not documented in the constructor of the orbit morphism.
Orbits are in 1:1 correspondence with cones of the original fan, so it makes perfect sense to pass this information and store it (as it is currently done in the patch).
Mathematically, this is all that is needed, but since we have so far issues with supporting quotient lattices and instead the fan of the orbit lives in a "regular lattice", we need to keep the correspondence somehow. As it is done by some matrix, perhaps that's what we need to pass to the constructor and store.
Instead, the current version constructs a codomain_ray>domain_ray dictionary, it is passed to the constructor and constructor reverses it into domain_ray>codomain_index dictionary. Note that the map does not have to be onetoone for nonsimplicial fans, so this dictionary just picks some random representative for a domain ray. The choice may affect the decision on whether embedding can be realized as a polynomial map or not.
I also think it is confusing to store nonprimitive generators for rays and treat a ray as not found if a nonprimitive generator is found. We do represent rays throughout the code just by their generators, but it is always assumed that they are normalized.
As a feature request it would be nice to have support for maps given by homogeneous polynomials in the other direction, i.e. a map from the 12 orbit of P112 to P1 can be given as (0:z1:z2) > (z1^{2:z2) and this will work for all orbits with powers corresponding to that "nonprimitivity of generators". Or is it already implemented and I am missing something? }
Anyway, concretely: how about passing and storing defining_cone
and projection_matrix
as base pieces of data and relying on them for consecutive computations?
comment:6 Changed 10 years ago by
For the patchbot (which should read the description...)
Apply trac_12892_orbit_closure_morphism.patch, trac_12892_toric_morphism_fibers.patch, trac_12892_toric_morphism_divisors.patch
comment:7 Changed 10 years ago by
As discussed with Andrey, the existence of polynomial map doesn't depend on the choices made.
Updated patch to add some clarifications.
comment:8 Changed 10 years ago by
Keywords:  sd40.5 added 

comment:10 Changed 10 years ago by
For the second patch:
relative_star_generators
does not have INPUT/OUTPUT and in general it would be nice to have a clear description of what it does. Can we please rename
fiber
togeneric_fiber
? (I would expect thatfiber
would return a particular one based on some input.) Also  why the documentation says that it returns a connected component, isn't it unique for a generic fiber?  I also got confused by
fiber_component
name thinking it computes the fiber over points corresponding to higherdimensional cones of the codomain. After some more thinking and reading I think that it is indeed the correct name, but would be nice to describe in the documentation the structure of nongeneric fibers and why it makes more sense to work with components corresponding to domain cones rather than fibers of codomain ones. fiber_component
andfiber_dimension
also lack INPUT/OUTPUT blocks.SchemeMorphism_fan_fiber_toric_variety
input documentation does not match the code. Perhaps the name of the class can be changed to
..._fiber_component_...
since it does not operate with the whole fiber.  "Defined by embedding the fiber irreducible component defined by the primitive preimage cone 1d cone of Rational polyhedral fan in 4d lattice N." does not read. While I was trying to reformulate it, I became unsure of this class at all. Isn't it just about embedding a torus orbit closure into the original toric variety? I.e. the toric morphism and fibers are not important?
comment:11 Changed 10 years ago by
Dependencies:  #12361 → #12361, #13023 

Status:  needs_review → needs_work 
Work issues:  → comments and rebasing 
comment:12 Changed 10 years ago by
I've updated the paths for #13023, and checked that all doctests pass.
comment:13 Changed 10 years ago by
Description:  modified (diff) 

Beginning of the reviewer patch: I changed fiber
to fiber_generic
and want to change fiber_component
to something more accurate since it does not necessarily return an irreducible component of a fiber, but perhaps a smaller dimensional piece. How about fiber_orbit_closure
? That's how the documentation describes the output anyway.
comment:14 followup: 48 Changed 10 years ago by
sage: P1 = toric_varieties.P1() sage: f = P1.hom(matrix([2]), P1) sage: f.fiber_orbit_closure(P1.fan(1)[0]) Traceback (most recent call last): ... ArithmeticError: Argument gens (= [(1/2)]) does not generate a submodule of self.
This should either give a meaningful error message or better yet just work. I'll try to take care of it.
comment:15 Changed 10 years ago by
The problem is of course that z > z^2
has two points as the fiber over {+1}. This is called the index in HuLiuYau. So we can either
a) Ignore it and just return one irreducible component
b) Return a pair
(toric variety, index)
c) Return the different connected components
(fiber, fiber, ..., fiber)
. Note that the embedding map differs by some roots of unity so you may not be able to write it over QQ...
comment:16 Changed 10 years ago by
Yeap, I am working on it  had to brush up the math definitions and then a nontrivial random example exposed some issues with current code which I am fixing. Regarding returned values, I didn't make up my mind yet, but just ignoring other components is a bad choice, I think  even if we return only one there should be another method that will return their number (and I almost have it working, I think).
comment:17 Changed 10 years ago by
Thinking about it, returning a pair (fiber, index)
or perhaps a dict seems like the best choice. Having to call another method to get the index isn't easy to discover.
comment:18 Changed 10 years ago by
There were failures due to the switch to PointCollection
, I've fixed them (added __add__
).
How about the following?
fiber_generic()
returns (X, N) where X is a toric variety corresponding to the kernel fan and N is the number of copies if the whole torus of the codomain is covered surjectively and 0 otherwise.fiber_component(domain_cone)
returns (X, N) where N is always some positive number of copies, since here we specify a domain cone and there are definitely some components corresponding to it, in particularfiber_component(domain_origin)
will return the number of components of the fiber over distinguished point of thecodomain_origin
, even iffiber_generic
returns (X, 0), meaning that over nondistinguished points there are likely empty fibers.fiber_dimension(codomain_cone)
returns 1 (or intinity?) if the corresponding orbit of the codomain is not covered surjectively.fiber_graph(codomain_cone)
returns an empty graph is the corresponding orbit is not covered surjectively.
Changed 10 years ago by
Attachment:  trac_12892_reviewer.patch added 

comment:19 followup: 25 Changed 10 years ago by
I (still) think its unwieldy to try to support nonsurjective "fibrations". A fibration is a surjective morphism, otherwise its useless to describe "fibers" fitting into some familiy parametrized by the base. Its would be much easier to just raise some error in fiber_...()
methods if the morphism is not surjective.
It seems like there is a natural way to factor a morphism into a surjection and an injection, first map to the image fan and then embed the image in the codomain. The error raised in the fiber_...()
methods could direct the users then to do this.
comment:20 followup: 24 Changed 10 years ago by
Well, what exactly do you mean by "fibration"? Fiber bundle? Fibration in the sense of is_fibration
with all fibers having the same dimension? That excludes even blowups. Anything surjective? That excludes e.g. chart inclusion, or maps from charts of blowups into the original variety. I think for any morphism it is reasonable to look at fibers over any point of the codomain, it is just the inverse image. And whether we return something special or raise an exception, we still have to do the work of figuring out what's going on.
From the user point of view, I think it is a bit easier/more elegant to handle "corner case" outputs than exceptions, fiber_component
and fiber_dimension
have pretty clear meaning in general. I think fiber_generic
has it as well. The fiber_graph
one is indeed quite special and maybe it makes sense to restrict it to surjective or even is_fibration
case.
In the case of factoring through the image, the middle variety can be difficult to work with and I am not even sure if the current support is sufficient for any work as well. Then again for the inclusion there is a question of what is covered and what is not, and the natural test seems to be  are there points of this point or not, but if fiber methods don't work, then there should be something separate. I think it will be more complicated.
E.g. take a blowup of a plane and consider the map from one of the charts to the plane. How will it factor??? I suspect clear factorization into surjection and injection requires working with complete fans and that's definitely way too restrictive.
comment:21 Changed 10 years ago by
Thinking some more about the last example (which I think we should support for sure): am I right that the image is a toric variety, but not normal, so there is no fan corresponding to it? As I just checked, restrict_to_image
so far is a method of fan morphisms only and using it here does nothing. I think this has to be the case, but the documentation should be clarified as the restriction does not have to be surjective. It kind of contradicts the name though, should we rename it? I think it is restrict_to_image_closure
inside of the codomain. We can rename the current method to it and then have restrict_to_image
which will do the same, but then check that the result is surjective and raise ValueError
otherwise since "proper image restriction" is no longer a fan morphism.
comment:22 Changed 10 years ago by
There isn't really a universally accepted definition of "fibration", it depends on the category you are in. In algebraic topology it is a morphism with the homotopy lifting property, but in algebraic geometry the underlying topological space of a fibration usually is not a fibration in the topology sense. As another data point, Danilov&Shafarevich define a fibration of curves to be onto.
Also, elliptic fibration definitely implies surjective to me. Or, in other words, if you excise complete preimages in the codomain then this is not what I would call an elliptic fibration. Though of course YMMV.
Regardless of the category, I think "fibration" ought to mean that one is not dealing with the most general morphism but with a suitable morphism such that the preimages can be thought of as being parametrized by the base. To me, this means that you definitely want the morphism to be surjective or at least dominant.
In algebraic geometry, a fibration needs not be equidimensional (or flat in the sense of commutative algebra). This is different from the topologist's fibrations.
comment:23 Changed 10 years ago by
Well, I guess what I suggest boils down to:
 fibration  a surjective equidimensional morphism, a "global in codomain" property check by
is_fibration
;  fiber  a preimage of a single point, a "local in codomain" construction welldefined for any morphism, regardless of its relation to fibrations in any category. http://mathworld.wolfram.com/Fiber.html is an easy to find reference, although not as authoritative as Danilov&Shafarevich ;)
Maybe it is a bit unfortunate that they have the same root, kind of like reduced/reducible, but calling it preimage_component(domain_cone)
is not clear enough since then it may mean full preimage of an orbit or its closure, and something like point_preimage_component(domain_cone)
is incomprehensible. Limiting to surjective morphisms and excluding a morphism from a blowup chart just because of vocabulary issues is a bit unsatisfactory...
comment:24 Changed 10 years ago by
Replying to novoselt:
E.g. take a blowup of a plane and consider the map from one of the charts to the plane. How will it factor???
I don't understand what the problem is. This should factor into the inclusion of the one chart in the blowup (injective), and the blowdown morphism (surjective). That is:
Fan([ Cone([(1,0),(1,1)]) ]) > Fan([ Cone([(1,0),(1,1)]), Cone([(1,1),(0,1)]) ]) > Fan([ Cone([(1,0),(0,1)]) ])
comment:25 Changed 10 years ago by
Replying to vbraun:
It seems like there is a natural way to factor a morphism into a surjection and an injection, first map to the image fan and then embed the image in the codomain.
That's what you have suggested earlier and this is also the factorization used in HLY paper. The intermediate variety is clearly defined: intersect the codomain fan with the linear subspace spanned by the map of lattices. Works fine when both fans of domain/codomain are complete.
Now for the blowup chart you propose to replace a map between two affine varieties induced by the identity matrix with a reverse factorization  first inclusion, then surjection, going through an intermediate nonaffine variety. How is this intermediate variety defined/constructed in general?
comment:26 Changed 10 years ago by
Sorry for the long hiatus, but I'm ready to finish this ticket now.
I think the problem is a question of language, the HLY factorization is into a surjective map and one that is generically injective (but not necessarily everywhere if the domain fan is not complete). So thats fine, we'll implement this factorization and then attach the functionaly of this ticket to the surjective fan morphisms. Do you agree?
comment:27 Changed 10 years ago by
Dependencies:  #12361, #13023 → #12361, #13023, #14353 

The factoring part is now #14353
comment:28 Changed 10 years ago by
I'm the one who should be sorry for delays and dragging tickets ;) I need to go over the discussion here and patches again as I remember confusing myself half a year ago while doing and undoing changes in my patch.
This factorization makes sense: first to the fan of images of cones in the image sublattice, then inclusion of this fan by the identity map, which is certainly injective on the torus, but may not be on the lower dimensional orbits. I'll try to get back with more detailed thoughts on Tuesday or, at worst, next weekend.
comment:29 Changed 10 years ago by
OK, just before the long weekend is over: I still would like to have methods for fibers work in nonsurjective case, but with surjective/genericallyinjective factorization it certainly does not have to be the default and perhaps can be turned on by some parameter like over_distinguished_point=True
which can then add extra output components as well.
comment:30 Changed 10 years ago by
My plan was to move all fibrationrelated stuff into a subclass for surjective toric morphisms. That'll allow us to have a simple interface for fibrations. This avoids the whole issue of selecting different fibers over various strata of the torus orbits in the codomain, which is encoded in the nonsurjective part of the factored morphism.
comment:32 Changed 9 years ago by
How about this compromise between surjective/general morphisms support  compute fibers only for dominant ones. For a toric morphism being dominant implies:
 surjection between tori, thus
 surjection onto any torus orbit if it was hit at all, so
 fiber/preimage over any point of any orbit is the same, including the case when it is empty.
So there is no confusion with returning something when distinguished points have something over them and some others don't. In terms of #14353 factorization, this means supporting compositions of surjective and birational factors, but throwing exceptions if the last injective factor is not an identity map on tori/lattices.
In particular, a blowup chart will be supported with single point fiber over the torus, single point fiber over one of the axis, empty fiber over another axis, and an affine line fiber over the origin. Awesome example for some intro classes ;)
comment:33 Changed 9 years ago by
There is some fuzz for the first patch on 511.beta3:
novoselt@sage:~/sage/devel/sage$ hg qseries trac_14210_matrix_mpolynomial_dense.patch trac_14210_reviewer.patch trac_13458_toric_Weierstrass_covering.patch trac_13458_reviewer.patch trac_12892_orbit_closure_morphism.patch trac_12892_toric_morphism_fibers.patch trac_12892_toric_morphism_divisors.patch novoselt@sage:~/sage/devel/sage$ hg qgoto ism_div applying trac_14210_matrix_mpolynomial_dense.patch applying trac_14210_reviewer.patch applying trac_13458_toric_Weierstrass_covering.patch applying trac_13458_reviewer.patch applying trac_12892_orbit_closure_morphism.patch patching file sage/schemes/toric/morphism.py Hunk #2 succeeded at 348 with fuzz 2 (offset 8 lines). applying trac_12892_toric_morphism_fibers.patch applying trac_12892_toric_morphism_divisors.patch now at: trac_12892_toric_morphism_divisors.patch
comment:34 Changed 9 years ago by
And somehow these patches break fresh factoring: 3 failures with AttributeError: 'SchemeMorphism_fan_toric_variety' object has no attribute 'is_birational'
comment:35 Changed 9 years ago by
Branch:  → u/vbraun/toric_fibration 

Description:  modified (diff) 
Work issues:  comments and rebasing → comments 
I moved the patches into a git branch and cleaned up the doctest errors (e.g. is_birational
was only defined for the fan morphism, not the scheme morphism).
comment:36 followup: 37 Changed 9 years ago by
Do we already have instructions somewhere on how to work with these branches?
comment:37 Changed 9 years ago by
comment:38 Changed 9 years ago by
Status:  needs_work → needs_review 

I've updated the branch to implement is_dominant
, a subclass of dominant toric morphisms, and moved the fibration methods there. Also, a subsection in the documentation to expand on your example of the blowup chart.
comment:39 Changed 9 years ago by
Milestone:  sage5.11 → sage5.12 

comment:40 Changed 9 years ago by
Milestone:  sage5.12 → sage6.0 

comment:41 Changed 9 years ago by
Commit:  → c3357583cf90021b906c52e635a9b9793e9234c9 

Branch pushed to git repo; I updated commit sha1. New commits:
[changeset:c335758]  resolved conflict with Trac #14891 
comment:42 Changed 9 years ago by
Will change return format to (fiber_component, index)
to play with git.
comment:43 Changed 9 years ago by
Cc:  Jan Keitel added 

comment:44 Changed 9 years ago by
Branch:  u/vbraun/toric_fibration → u/novoselt/toric_fibration 

comment:45 Changed 9 years ago by
Commit:  c3357583cf90021b906c52e635a9b9793e9234c9 → f0aedcd605f8328e7e77ed633e72f57f106aeae1 

Branch pushed to git repo; I updated commit sha1. New commits:
[changeset:f0aedcd]  Make fiber_generic return component count, doctests not adjusted yet. 
[changeset:c335758]  resolved conflict with Trac #14891 
[changeset:19e832a]  Trac #12892: Move fibration methods to subclass of dominant morphisms 
[changeset:1142feb]  Trac #12892: Reviewer changes to toric morphisms. 
[changeset:78dc617]  Trac #12892: Pull back toric divisors via toric morphisms 
[changeset:7fb4df5]  Trac #12892: Toric fibration fiber 
[changeset:9aa485f]  Trac #12892: Toric fibration morphisms 
comment:46 Changed 9 years ago by
Commit:  f0aedcd605f8328e7e77ed633e72f57f106aeae1 → 363f57b548841f4d272bf8417fb114b6509324d0 

Branch pushed to git repo; I updated commit sha1. New commits:
[changeset:363f57b]  Doctests pass, still looking at code. 
comment:47 Changed 9 years ago by
Commit:  363f57b548841f4d272bf8417fb114b6509324d0 → 943e71bbe836b097875746717b0621e5ac04e120 

Branch pushed to git repo; I updated commit sha1. New commits:
[changeset:943e71b]  Use span instead of submodule to deal with rational preimages 
[changeset:1949102]  Throw ValueError? instead of TypeError? for complicated morphisms. 
comment:48 Changed 9 years ago by
Made square map work. Also tried to use quotient lattices more but without much success.
What exactly is _image_ray_multiplicity
mathematically? I am confused by picking the maximum one  why don't other matter?
comment:49 Changed 9 years ago by
Commit:  943e71bbe836b097875746717b0621e5ac04e120 → 6b4b318ad94b078d4eb0323d7b72a96c60e4c040 

Branch pushed to git repo; I updated commit sha1. New commits:
[changeset:6b4b318]  Treat generic fiber correctly as a fiber component. 
[changeset:317dd40]  Fixes to deal with maps between sublattices and virtual rays. 
comment:50 Changed 9 years ago by
Thinking of making aris
(a variant of spelling arris, "the sharp edge or salient angle formed by the meeting of two surfaces especially in moldings") an alias for ambient_ray_indices
;) Would make doctest a bit shorter.
Still have a bug in fan isomorphism and another one in representing morphisms as polynomial maps. Problems tend to stem from either fans/morphisms involving sublattices, which seem to be easy to fix, or from not dealing properly with torus factors, which are more involved.
comment:51 Changed 9 years ago by
Commit:  6b4b318ad94b078d4eb0323d7b72a96c60e4c040 → e79e8405a5c762eeaed7b6553195f6c73d561861 

Branch pushed to git repo; I updated commit sha1. New commits:
[changeset:e79e840]  Added a long fibration example. One doctest failure due to fan isomorphism bug. 
comment:52 Changed 9 years ago by
Jan  I think that embedding construction code here is stabilized (unless Volker has any objections) and it is OK to merge your changes on top of this.
comment:53 Changed 9 years ago by
Volker  How about adding .coordinates()
or something like this method to PointCollection
which will return a point collection with the same points as the original, but written in terms of the basis of the original module? I think this will help in dealing with sublattices and quotients, e.g. there is a bunch of methods for 2d fans that test dimension of the lattice, but need to take into account that actual points may have more than 2 components. This is the reason for the failing doctest where P2 is represented by a fan in a sublattice.
comment:54 Changed 9 years ago by
Keywords:  sd53 added 

Work issues:  comments → fan isomorphism bug 
comment:55 Changed 9 years ago by
Milestone:  sage6.0 → sage6.1 

comment:56 Changed 9 years ago by
Hey Volker, since you have started merging branches, I propose marking the failing doctest for P2 as known bug and merging this one as well  correct fix for P2 will require some careful work and it is certainly not introduced by this ticket. Do you have any objections to my changes so far?
comment:57 Changed 9 years ago by
I agree with your changes, thanks! I'm also fine with marking the P2 thing as known bug and split it of to a separate ticket. Please go ahead!
comment:58 Changed 9 years ago by
Milestone:  sage6.1 → sage6.2 

comment:59 Changed 9 years ago by
Commit:  e79e8405a5c762eeaed7b6553195f6c73d561861 → b3f06ed5bb902f1f4ce0f3f11935cf62101137d0 

Branch pushed to git repo; I updated commit sha1. New commits:
b3f06ed  Marked failing doctest as a known bug #16012.

comment:60 Changed 9 years ago by
Keywords:  toric added 

Status:  needs_review → positive_review 
Work issues:  fan isomorphism bug 
Hey Volker  sorry for spending 3 months on these 3 doc lines, but it is still seems to be mergeable and tests run fine for me. I'll take the liberty to switch to positive review since this change was preapproved.
comment:61 Changed 9 years ago by
Branch:  u/novoselt/toric_fibration → b3f06ed5bb902f1f4ce0f3f11935cf62101137d0 

Resolution:  → fixed 
Status:  positive_review → closed 
This looks very cool, I plan to go over details in a couple weeks or sooner.