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: sage-6.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:

Status badges

Description (last modified by Volker Braun)

This ticket provides more morphisms that are associated to toric varieties:

  • embedding of an orbit closure
  • embedding of a fiber of a toric morphism
  • pull-back of divisors

Use the git branch!

Attachments (4)

trac_12892_orbit_closure_morphism.patch (11.2 KB) - added by Volker Braun 10 years ago.
Updated patch
trac_12892_reviewer.patch (10.7 KB) - added by Andrey Novoseltsev 10 years ago.
trac_12892_toric_morphism_divisors.patch (7.8 KB) - added by Volker Braun 9 years ago.
Updated patch
trac_12892_toric_morphism_fibers.patch (26.2 KB) - added by Volker Braun 9 years ago.
Updated patch

Download all attachments as: .zip

Change History (65)

comment:1 Changed 10 years ago by Volker Braun

Authors: Volker Braun
Cc: Andrey Novoseltsev added
Description: modified (diff)
Status: newneeds_review

comment:2 Changed 10 years ago by Andrey Novoseltsev

Reviewers: Andrey Novoseltsev

This looks very cool, I plan to go over details in a couple weeks or sooner.

comment:3 Changed 10 years ago by Volker Braun

In 3 weeks we can also talk about it in person in Seattle :-)

comment:4 Changed 10 years ago by Andrey Novoseltsev

Dependencies: #12361

comment:5 Changed 10 years ago by Andrey Novoseltsev

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 one-to-one for non-simplicial 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 non-primitive generators for rays and treat a ray as not found if a non-primitive 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) -> (z12:z2) and this will work for all orbits with powers corresponding to that "non-primitivity 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 Andrey Novoseltsev

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 Volker Braun

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 Volker Braun

Keywords: sd40.5 added

comment:9 Changed 10 years ago by Andrey Novoseltsev

OK to the first patch!

comment:10 Changed 10 years ago by Andrey Novoseltsev

For the second patch:

  1. relative_star_generators does not have INPUT/OUTPUT and in general it would be nice to have a clear description of what it does.
  2. Can we please rename fiber to generic_fiber? (I would expect that fiber 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?
  3. I also got confused by fiber_component name thinking it computes the fiber over points corresponding to higher-dimensional 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 non-generic fibers and why it makes more sense to work with components corresponding to domain cones rather than fibers of codomain ones.
  4. fiber_component and fiber_dimension also lack INPUT/OUTPUT blocks.
  5. SchemeMorphism_fan_fiber_toric_variety input documentation does not match the code.
  6. Perhaps the name of the class can be changed to ..._fiber_component_... since it does not operate with the whole fiber.
  7. "Defined by embedding the fiber irreducible component defined by the primitive preimage cone 1-d cone of Rational polyhedral fan in 4-d 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 Andrey Novoseltsev

Dependencies: #12361#12361, #13023
Status: needs_reviewneeds_work
Work issues: comments and rebasing

Changed 10 years ago by Volker Braun

Updated patch

comment:12 Changed 10 years ago by Volker Braun

I've updated the paths for #13023, and checked that all doctests pass.

comment:13 Changed 10 years ago by Andrey Novoseltsev

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 Changed 10 years ago by Andrey Novoseltsev

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 Volker Braun

The problem is of course that z |-> z^2 has two points as the fiber over {+1}. This is called the index in Hu-Liu-Yau. 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 Andrey Novoseltsev

Yeap, I am working on it - had to brush up the math definitions and then a non-trivial 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 Volker Braun

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 Andrey Novoseltsev

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 particular fiber_component(domain_origin) will return the number of components of the fiber over distinguished point of the codomain_origin, even if fiber_generic returns (X, 0), meaning that over non-distinguished 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 Andrey Novoseltsev

Attachment: trac_12892_reviewer.patch added

comment:19 Changed 10 years ago by Volker Braun

I (still) think its unwieldy to try to support non-surjective "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 Changed 10 years ago by Andrey Novoseltsev

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 Andrey Novoseltsev

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 Volker Braun

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 Andrey Novoseltsev

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 well-defined 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 in reply to:  20 Changed 10 years ago by Volker Braun

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 blow-up (injective), and the blow-down 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 in reply to:  19 Changed 10 years ago by Andrey Novoseltsev

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 non-affine variety. How is this intermediate variety defined/constructed in general?

comment:26 Changed 10 years ago by Volker Braun

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 Volker Braun

Dependencies: #12361, #13023#12361, #13023, #14353

The factoring part is now #14353

comment:28 Changed 10 years ago by Andrey Novoseltsev

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 Andrey Novoseltsev

OK, just before the long weekend is over: I still would like to have methods for fibers work in non-surjective case, but with surjective/generically-injective 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 Volker Braun

My plan was to move all fibration-related 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 non-surjective part of the factored morphism.

Changed 9 years ago by Volker Braun

Updated patch

Changed 9 years ago by Volker Braun

Updated patch

comment:31 Changed 9 years ago by Volker Braun

Rediffed for sage-5.11.beta3

comment:32 Changed 9 years ago by Andrey Novoseltsev

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 Andrey Novoseltsev

There is some fuzz for the first patch on 5-11.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 Andrey Novoseltsev

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 Volker Braun

Branch: u/vbraun/toric_fibration
Description: modified (diff)
Work issues: comments and rebasingcomments

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 Changed 9 years ago by Andrey Novoseltsev

Do we already have instructions somewhere on how to work with these branches?

comment:37 in reply to:  36 Changed 9 years ago by Volker Braun

Replying to novoselt:

Do we already have instructions somewhere on how to work with these branches?

Working on it at #14481

comment:38 Changed 9 years ago by Volker Braun

Status: needs_workneeds_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 Jeroen Demeyer

Milestone: sage-5.11sage-5.12

comment:40 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.12sage-6.0

comment:41 Changed 9 years ago by git

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 Andrey Novoseltsev

Will change return format to (fiber_component, index) to play with git.

comment:43 Changed 9 years ago by Jan Keitel

Cc: Jan Keitel added

comment:44 Changed 9 years ago by Andrey Novoseltsev

Branch: u/vbraun/toric_fibrationu/novoselt/toric_fibration

comment:45 Changed 9 years ago by git

Commit: c3357583cf90021b906c52e635a9b9793e9234c9f0aedcd605f8328e7e77ed633e72f57f106aeae1

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 git

Commit: f0aedcd605f8328e7e77ed633e72f57f106aeae1363f57b548841f4d272bf8417fb114b6509324d0

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 git

Commit: 363f57b548841f4d272bf8417fb114b6509324d0943e71bbe836b097875746717b0621e5ac04e120

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 in reply to:  14 Changed 9 years ago by Andrey Novoseltsev

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 git

Commit: 943e71bbe836b097875746717b0621e5ac04e1206b4b318ad94b078d4eb0323d7b72a96c60e4c040

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 Andrey Novoseltsev

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 git

Commit: 6b4b318ad94b078d4eb0323d7b72a96c60e4c040e79e8405a5c762eeaed7b6553195f6c73d561861

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 Andrey Novoseltsev

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 Andrey Novoseltsev

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 Andrey Novoseltsev

Keywords: sd53 added
Work issues: commentsfan isomorphism bug

comment:55 Changed 9 years ago by For batch modifications

Milestone: sage-6.0sage-6.1

comment:56 Changed 9 years ago by Andrey Novoseltsev

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 Volker Braun

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 For batch modifications

Milestone: sage-6.1sage-6.2

comment:59 Changed 9 years ago by git

Commit: e79e8405a5c762eeaed7b6553195f6c73d561861b3f06ed5bb902f1f4ce0f3f11935cf62101137d0

Branch pushed to git repo; I updated commit sha1. New commits:

b3f06edMarked failing doctest as a known bug #16012.

comment:60 Changed 9 years ago by Andrey Novoseltsev

Keywords: toric added
Status: needs_reviewpositive_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 Volker Braun

Branch: u/novoselt/toric_fibrationb3f06ed5bb902f1f4ce0f3f11935cf62101137d0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.