Opened 12 years ago

Closed 11 years ago

#11200 closed enhancement (fixed)

Add fibration check to FanMorphism

Reported by: Andrey Novoseltsev Owned by: mhampton
Priority: major Milestone: sage-4.7.1
Component: geometry Keywords: toric
Cc: Volker Braun Merged in: sage-4.7.1.alpha3
Authors: Andrey Novoseltsev Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #10140, #10882 Stopgaps:

Status badges

Description (last modified by Andrey Novoseltsev)

The main point of this ticket was to add FanMorphism.is_fibration method, but along the way injectivity, surjectivity, and splitting were added as well.

I have also added several related methods and adjusted containment/embedding methods of fans to accommodate sublattices and make their behaviour more uniform: now cone containment check is an attempt to embed the given cone into the fan.

I have also started using @cached_method decorator since it is convenient and its speed and documentation issues should be resolved in the near future. Note, however, that some methods of fans still should use manual cache since they need to preprocess arguments first.

Attachments (1)

trac_11200_Add_fibration_check_to_FanMorphism.patch (38.8 KB) - added by Andrey Novoseltsev 12 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 12 years ago by Andrey Novoseltsev

Status: newneeds_review

comment:2 Changed 12 years ago by Andrey Novoseltsev

Dependencies: #10140, #10882
Description: modified (diff)

Rebased on top of new #10882.

comment:3 Changed 12 years ago by Volker Braun

Reviewers: Volker Braun

I like the functionality, but you need to check surjectivity of the map of toric varieties, not just the map of vector spaces. The is_surjective() method should do that but is broken:

sage: P1 = toric_varieties.P1()
sage: A1 = toric_varieties.A1()
sage: phi = FanMorphism(matrix([[1]]),,
sage: phi.is_fibration()
sage: phi.is_surjective()

But thats neither a fibration nor a surjective map.

comment:4 Changed 12 years ago by Volker Braun

Status: needs_reviewneeds_work

comment:5 Changed 12 years ago by Andrey Novoseltsev

Hmmm, I thought that it does check support surjection, will look into it.

As for is_surjective, it is wrong because it is inherited. I will add an appropriate toric implementation to this patch.

comment:6 Changed 12 years ago by Andrey Novoseltsev

Status: needs_workneeds_info

Hi Volker,

I fixed the mistake you have found and wrote is_surjective.

While I am at it, I also started writing is_injective, but it is a bit trickier: we want the lattice map to have index one in the saturation of its image (in Sage, conveniently, self.matrix().index_in_saturation() == 1) and for every (primitive) cone tau mapped to sigma the dimension of tau must be the same as the dimension of the intersection of sigma with the image lattice of the lattice map. To make this easier and potentially simplify life in the future with computing fibers of generic fan/toric morphisms, I am thinking of adding the following methods:

  1. image_lattice to be the sublattice generated by the image of the domain lattice.
  2. image_fan to be the intersection of the codomain fan with the image lattice.
  3. image_restriction to be the fan morphism from the original domain fan to the newly defined image fan.

Question: how should we name these methods? It seems to me that image_lattice fits nicely with kernel_lattice, but two others may be quite confusing/misleading, i.e. image_fan may refer to the image of the domain fan in the image lattice. One possible solution is not to implement such a method at all as it may be accessed via


Any thoughts on this and the name for the image_restriction map?

comment:7 Changed 12 years ago by Andrey Novoseltsev

Update: actually, just image already does what I called image_lattice above, there is also kernel that seems to do kernel_lattice job, so maybe image/kernel are sufficient. Then the only issue is image_restriction!

comment:8 Changed 12 years ago by Andrey Novoseltsev

And there is also restrict_codomain, similar in spirit, although I probably want to cache this special "natural" restriction.

comment:9 Changed 12 years ago by Volker Braun

Since our naming theme is to use method() for the inherited method on vector spaces and method_fan() for the induced function of the fan, how about restrict_domain_fan() and restrict_codomain_fan(). The restriction to the image would then be

phi.restrict_codomain_fan( phi.image_fan() )

For the restriction of the vector space map you have to use this construction as well, I think; as far as I can tell there is no method that restricts to the image in one go.

Also, I don't think image_fan() is overly confusing. After all, the fan is a collection of cones over \QQ.

For injectivity you need to check that the index of each domain cone is one (Def 2.1.7 in HLY). Are you going to override the FanMorphism.index() method? I would have thought that this is a logical place to put this functionality.

comment:10 Changed 12 years ago by Andrey Novoseltsev

Yes, phi.index(sigma) is the way to go, I think, but having these indices equal to 1 is not enough (and they are all 1 if the linear map has index one in its image saturation). We also need "relative stars" to be trivial, so that in the notation of Proposition 2.1.4 in HLY \tld{O}_\sigma is a 1-to-1 cover and F_\sigma^c is a singleton.

Going back to names, I will try to be more careful in the future with checking inherited methods, but it seems to me now that

  • kernel_lattice should be removed since kernel computes exactly the same thing using generic code.
  • image_lattice should not be implemented since phi.image().saturation() does what I want and plane image() is also of interest in the toric setting. (Currently, saturation will not return a toric sublattice, but it is easy to fix.)

For the factoring of the fan morphism phi: Sigma ---> Sigma' we (always) have the following sequence of fan maps:

Sigma0 --0--> Sigma --1--> Sigma1 --2--> Sigma2 --3--> Sigma'

where Sigma0 is the kernel fan, Sigma1 is phi(Sigma) living in phi(N_RR) \cap N', and Sigma2 is phi(N_RR) \cap Sigma'. Map 1 is easy to construct as

FanMorphism(phi.matrix(), phi.domain_fan(), phi.image().saturation())

Maps 2 and 3 are basically given by identity matrix but there is currently no "one line way" to construct Sigma2 which I want to add. Now what I was referring to, is that both Sigma1 and Sigma2 are natural candidates to be called image_fan since they are fans in the image of phi and perhaps Sigma1 is more natural since it is literally the image fan of Sigma under phi.

Since "general" restrictions of phi to other fans can be easily done using FanMorphism constructor directly, I am not sure there is any need in adding special methods to restrict (co)domain to a provided fan. So I think I am in favor of adding only, say, restrict_to_image() with no arguments which will be the composition of maps 1 and 2 in the diagram. The main job of this function will be, of course, efficient construction of Sigma2 and once it is done all maps of the sequence can be easily constructed if needed.

Summary of the proposal:

  1. remove kernel_lattice and use inherited kernel instead;
  2. tweak saturation to make phi.image().saturation() to return a toric sublattice;
  3. add restrict_to_image returning the map Sigma ---> Sigma2;
  4. let phi.index(sigma) take a cone as an argument and return its index in the sense of HLY.

Sounds good?

comment:11 Changed 12 years ago by Andrey Novoseltsev

Work issues: sublattice duals and action

Update: 1 and 2 are easy and done, 3 is in principle also easy and done, but it does not yet work due to sublattice handling. So I am working on improving it. Ideally I want to be able to do in sublattices and quotient lattices everything that can be done in the ambient ones. Several steps in this direction were already done, this patch will make a few more ;-)

comment:12 Changed 12 years ago by Andrey Novoseltsev

Description: modified (diff)
Status: needs_infoneeds_review
Work issues: sublattice duals and action

OK, I propose this version for inclusion. I did not add index(cone) functionality as it is not necessary for the current methods and it would make sense only for those morphisms whose vector space map is surjective (there should be perhaps another is_xxx method to test for this, but I am not sure how to call it - is_dense?). Besides, someone else is likely to work on further description of fan morphism fibers and it will be a nice exercise to add cone indices ;-)

I made a small change to free_module, but didn't yet test consequences on the whole library. I'll do it today and will fix any problems if there are any.

comment:13 Changed 12 years ago by Andrey Novoseltsev

Fixed a couple of typos in the documentation and removed "not tested" comment in the kernel_fan as it is now possible to construct its cone lattice.

comment:14 Changed 12 years ago by Andrey Novoseltsev

While patchbot made a complete streetlight thinking that this patch get worse and worse, direct testing shows that all long tests pass on top of sage-4.7.rc2 with dependencies applied!

comment:15 Changed 12 years ago by Volker Braun

Looks all good to me.

Just a minor nitpick: I think that is_splitting() is very unintuitive. Does anyone besides HLY use that notation? How about we rename it to is_bundle() or at least make an alias?

comment:16 Changed 12 years ago by Andrey Novoseltsev

Status: needs_reviewneeds_work
Work issues: splitting ---> bundle

That was actually from "Toric Varieties" book. But I like is_bundle more, so I'll change it. Thanks for the suggestion!

comment:17 Changed 12 years ago by Andrey Novoseltsev

Status: needs_workneeds_review
Work issues: splitting ---> bundle


comment:18 Changed 12 years ago by Volker Braun

Status: needs_reviewpositive_review


The patch buildbot is confused about which patches from the dependency #10140 need to be applied. I tried to help the patch buildbot to figure it out but without success. All doctest pass on my machine.

comment:19 Changed 12 years ago by Volker Braun

Dependencies: #10140, #10882#10140, #10882, #9128

The documentation makes use of :trac:... sphinx tags and therefore depends on #9128.

Changed 12 years ago by Andrey Novoseltsev

comment:20 Changed 12 years ago by Andrey Novoseltsev

Dependencies: #10140, #10882, #9128#10140, #10882

Not anymore :-(

#9128 has been around for a while and it has further dependencies, I'd rather not delay this ticket till they all are resolved...

comment:21 Changed 12 years ago by Volker Braun

Great, ready to be included in any 4.7.1.alpha now!

comment:22 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-4.7.1.alpha3
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.