Sage: Ticket #11200: Add fibration check to FanMorphism
https://trac.sagemath.org/ticket/11200
<p>
The main point of this ticket was to add <code>FanMorphism.is_fibration</code> method, but along the way injectivity, surjectivity, and splitting were added as well.
</p>
<p>
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.
</p>
<p>
I have also started using <code>@cached_method</code> 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.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/11200
Trac 1.1.6novoseltThu, 14 Apr 2011 18:40:01 GMTstatus changed
https://trac.sagemath.org/ticket/11200#comment:1
https://trac.sagemath.org/ticket/11200#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
TicketnovoseltSun, 24 Apr 2011 19:18:49 GMTdescription changed; dependencies set
https://trac.sagemath.org/ticket/11200#comment:2
https://trac.sagemath.org/ticket/11200#comment:2
<ul>
<li><strong>dependencies</strong>
set to <em>#10140, #10882</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/11200?action=diff&version=2">diff</a>)
</li>
</ul>
<p>
Rebased on top of new <a class="closed ticket" href="https://trac.sagemath.org/ticket/10882" title="enhancement: Add kernel fan to fan morphism (closed: fixed)">#10882</a>.
</p>
TicketvbraunThu, 05 May 2011 12:50:05 GMTreviewer set
https://trac.sagemath.org/ticket/11200#comment:3
https://trac.sagemath.org/ticket/11200#comment:3
<ul>
<li><strong>reviewer</strong>
set to <em>Volker Braun</em>
</li>
</ul>
<p>
I like the functionality, but you need to check surjectivity of the map of toric varieties, not just the map of vector spaces. The <code>is_surjective()</code> method should do that but is broken:
</p>
<pre class="wiki">sage: P1 = toric_varieties.P1()
sage: A1 = toric_varieties.A1()
sage: phi = FanMorphism(matrix([[1]]), A1.fan(), P1.fan())
sage: phi.is_fibration()
True
sage: phi.is_surjective()
True
</pre><p>
But thats neither a fibration nor a surjective map.
</p>
TicketvbraunThu, 05 May 2011 12:50:25 GMTstatus changed
https://trac.sagemath.org/ticket/11200#comment:4
https://trac.sagemath.org/ticket/11200#comment:4
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
TicketnovoseltThu, 05 May 2011 15:28:53 GMT
https://trac.sagemath.org/ticket/11200#comment:5
https://trac.sagemath.org/ticket/11200#comment:5
<p>
Hmmm, I thought that it does check support surjection, will look into it.
</p>
<p>
As for <code>is_surjective</code>, it is wrong because it is inherited. I will add an appropriate toric implementation to this patch.
</p>
TicketnovoseltSat, 07 May 2011 04:50:34 GMTstatus changed
https://trac.sagemath.org/ticket/11200#comment:6
https://trac.sagemath.org/ticket/11200#comment:6
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_info</em>
</li>
</ul>
<p>
Hi Volker,
</p>
<p>
I fixed the mistake you have found and wrote <code>is_surjective</code>.
</p>
<p>
While I am at it, I also started writing <code>is_injective</code>, but it is a bit trickier: we want the lattice map to have index one in the saturation of its image (in Sage, conveniently, <code>self.matrix().index_in_saturation() == 1</code>) and for every (primitive) cone <code>tau</code> mapped to <code>sigma</code> the dimension of <code>tau</code> must be the same as the dimension of the intersection of <code>sigma</code> 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:
</p>
<ol><li><code>image_lattice</code> to be the sublattice generated by the image of the domain lattice.
</li><li><code>image_fan</code> to be the intersection of the codomain fan with the image lattice.
</li><li><code>image_restriction</code> to be the fan morphism from the original domain fan to the newly defined image fan.
</li></ol><p>
Question: how should we name these methods? It seems to me that <code>image_lattice</code> fits nicely with <code>kernel_lattice</code>, but two others may be quite confusing/misleading, i.e. <code>image_fan</code> 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
</p>
<pre class="wiki">phi.image_restriction().codomain_fan()
</pre><p>
Any thoughts on this and the name for the <code>image_restriction</code> map?
</p>
TicketnovoseltSat, 07 May 2011 04:55:01 GMT
https://trac.sagemath.org/ticket/11200#comment:7
https://trac.sagemath.org/ticket/11200#comment:7
<p>
Update: actually, just <code>image</code> already does what I called <code>image_lattice</code> above, there is also <code>kernel</code> that seems to do <code>kernel_lattice</code> job, so maybe <code>image/kernel</code> are sufficient. Then the only issue is <code>image_restriction</code>!
</p>
TicketnovoseltSat, 07 May 2011 04:57:00 GMT
https://trac.sagemath.org/ticket/11200#comment:8
https://trac.sagemath.org/ticket/11200#comment:8
<p>
And there is also <code>restrict_codomain</code>, similar in spirit, although I probably want to cache this special "natural" restriction.
</p>
TicketvbraunSat, 07 May 2011 10:22:12 GMT
https://trac.sagemath.org/ticket/11200#comment:9
https://trac.sagemath.org/ticket/11200#comment:9
<p>
Since our naming theme is to use <code>method()</code> for the inherited method on vector spaces and <code>method_fan()</code> for the induced function of the fan, how about <code>restrict_domain_fan()</code> and <code>restrict_codomain_fan()</code>. The restriction to the image would then be
</p>
<pre class="wiki">phi.restrict_codomain_fan( phi.image_fan() )
</pre><p>
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.
</p>
<p>
Also, I don't think <code>image_fan()</code> is overly confusing. After all, the fan is a collection of cones over <code>\QQ</code>.
</p>
<p>
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 <code>FanMorphism.index()</code> method? I would have thought that this is a logical place to put this functionality.
</p>
TicketnovoseltSat, 07 May 2011 20:26:27 GMT
https://trac.sagemath.org/ticket/11200#comment:10
https://trac.sagemath.org/ticket/11200#comment:10
<p>
Yes, <code>phi.index(sigma)</code> 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 <code>\tld{O}_\sigma</code> is a 1-to-1 cover and <code>F_\sigma^c</code> is a singleton.
</p>
<p>
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
</p>
<ul><li><code>kernel_lattice</code> should be removed since <code>kernel</code> computes exactly the same thing using generic code.
</li><li><code>image_lattice</code> should not be implemented since <code>phi.image().saturation()</code> does what I want and plane <code>image()</code> is also of interest in the toric setting. (Currently, saturation will not return a toric sublattice, but it is easy to fix.)
</li></ul><p>
For the factoring of the fan morphism <code>phi: Sigma ---> Sigma'</code> we (always) have the following sequence of fan maps:
</p>
<pre class="wiki">Sigma0 --0--> Sigma --1--> Sigma1 --2--> Sigma2 --3--> Sigma'
</pre><p>
where <code>Sigma0</code> is the kernel fan, <code>Sigma1</code> is <code>phi(Sigma)</code> living in <code>phi(N_RR) \cap N'</code>, and <code>Sigma2</code> is <code>phi(N_RR) \cap Sigma'</code>. Map 1 is easy to construct as
</p>
<pre class="wiki">FanMorphism(phi.matrix(), phi.domain_fan(), phi.image().saturation())
</pre><p>
Maps 2 and 3 are basically given by identity matrix but there is currently no "one line way" to construct <code>Sigma2</code> which I want to add. Now what I was referring to, is that both <code>Sigma1</code> and <code>Sigma2</code> are natural candidates to be called <code>image_fan</code> since they are fans in the image of <code>phi</code> and perhaps <code>Sigma1</code> is more natural since it is literally the image fan of <code>Sigma</code> under <code>phi</code>.
</p>
<p>
Since "general" restrictions of <code>phi</code> to other fans can be easily done using <code>FanMorphism</code> 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, <code>restrict_to_image()</code> 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 <code>Sigma2</code> and once it is done all maps of the sequence can be easily constructed if needed.
</p>
<p>
Summary of the proposal:
</p>
<ol><li>remove <code>kernel_lattice</code> and use inherited <code>kernel</code> instead;
</li><li>tweak <code>saturation</code> to make <code>phi.image().saturation()</code> to return a toric sublattice;
</li><li>add <code>restrict_to_image</code> returning the map <code>Sigma ---> Sigma2</code>;
</li><li>let <code>phi.index(sigma)</code> take a cone as an argument and return its index in the sense of HLY.
</li></ol><p>
Sounds good?
</p>
TicketnovoseltTue, 17 May 2011 05:32:56 GMTwork_issues set
https://trac.sagemath.org/ticket/11200#comment:11
https://trac.sagemath.org/ticket/11200#comment:11
<ul>
<li><strong>work_issues</strong>
set to <em>sublattice duals and action</em>
</li>
</ul>
<p>
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 ;-)
</p>
TicketnovoseltWed, 18 May 2011 21:41:39 GMTstatus, description changed; work_issues deleted
https://trac.sagemath.org/ticket/11200#comment:12
https://trac.sagemath.org/ticket/11200#comment:12
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>work_issues</strong>
<em>sublattice duals and action</em> deleted
</li>
<li><strong>description</strong>
modified (<a href="/ticket/11200?action=diff&version=12">diff</a>)
</li>
</ul>
<p>
OK, I propose this version for inclusion. I did not add <code>index(cone)</code> 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 <code>is_xxx</code> method to test for this, but I am not sure how to call it - <code>is_dense</code>?). 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 ;-)
</p>
<p>
I made a small change to <code>free_module</code>, but didn't yet test consequences on the whole library. I'll do it today and will fix any problems if there are any.
</p>
TicketnovoseltWed, 18 May 2011 23:06:34 GMT
https://trac.sagemath.org/ticket/11200#comment:13
https://trac.sagemath.org/ticket/11200#comment:13
<p>
Fixed a couple of typos in the documentation and removed "not tested" comment in the <code>kernel_fan</code> as it is now possible to construct its cone lattice.
</p>
TicketnovoseltThu, 19 May 2011 00:23:39 GMT
https://trac.sagemath.org/ticket/11200#comment:14
https://trac.sagemath.org/ticket/11200#comment:14
<p>
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!
</p>
TicketvbraunSat, 28 May 2011 01:28:44 GMT
https://trac.sagemath.org/ticket/11200#comment:15
https://trac.sagemath.org/ticket/11200#comment:15
<p>
Looks all good to me.
</p>
<p>
Just a minor nitpick: I think that <code>is_splitting()</code> is very unintuitive. Does anyone besides HLY use that notation? How about we rename it to <code>is_bundle()</code> or at least make an alias?
</p>
TicketnovoseltSat, 28 May 2011 01:40:02 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/11200#comment:16
https://trac.sagemath.org/ticket/11200#comment:16
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>splitting ---> bundle</em>
</li>
</ul>
<p>
That was actually from "Toric Varieties" book. But I like <code>is_bundle</code> more, so I'll change it. Thanks for the suggestion!
</p>
TicketnovoseltSat, 28 May 2011 02:19:32 GMTstatus changed; work_issues deleted
https://trac.sagemath.org/ticket/11200#comment:17
https://trac.sagemath.org/ticket/11200#comment:17
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>work_issues</strong>
<em>splitting ---> bundle</em> deleted
</li>
</ul>
<p>
Done!
</p>
TicketvbraunSat, 28 May 2011 16:08:22 GMTstatus changed
https://trac.sagemath.org/ticket/11200#comment:18
https://trac.sagemath.org/ticket/11200#comment:18
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Nice!
</p>
<p>
The patch buildbot is confused about which patches from the dependency <a class="closed ticket" href="https://trac.sagemath.org/ticket/10140" title="enhancement: Base sage.geometry.cone on the Parma Polyhedra Library (PPL) (closed: fixed)">#10140</a> need to be applied. I tried to help the patch buildbot to figure it out but without success. All doctest pass on my machine.
</p>
TicketvbraunSat, 28 May 2011 20:09:19 GMTdependencies changed
https://trac.sagemath.org/ticket/11200#comment:19
https://trac.sagemath.org/ticket/11200#comment:19
<ul>
<li><strong>dependencies</strong>
changed from <em>#10140, #10882</em> to <em>#10140, #10882, #9128</em>
</li>
</ul>
<p>
The documentation makes use of <code>:trac:...</code> sphinx tags and therefore depends on <a class="closed ticket" href="https://trac.sagemath.org/ticket/9128" title="enhancement: Sphinx should be aware of all.py to find its links (closed: fixed)">#9128</a>.
</p>
TicketnovoseltSat, 28 May 2011 20:56:44 GMTattachment set
https://trac.sagemath.org/ticket/11200
https://trac.sagemath.org/ticket/11200
<ul>
<li><strong>attachment</strong>
set to <em>trac_11200_Add_fibration_check_to_FanMorphism.patch</em>
</li>
</ul>
TicketnovoseltSat, 28 May 2011 21:14:29 GMTdependencies changed
https://trac.sagemath.org/ticket/11200#comment:20
https://trac.sagemath.org/ticket/11200#comment:20
<ul>
<li><strong>dependencies</strong>
changed from <em>#10140, #10882, #9128</em> to <em>#10140, #10882</em>
</li>
</ul>
<p>
Not anymore :-(
</p>
<p>
<a class="closed ticket" href="https://trac.sagemath.org/ticket/9128" title="enhancement: Sphinx should be aware of all.py to find its links (closed: fixed)">#9128</a> has been around for a while and it has further dependencies, I'd rather not delay this ticket till they all are resolved...
</p>
TicketvbraunSat, 28 May 2011 22:15:56 GMT
https://trac.sagemath.org/ticket/11200#comment:21
https://trac.sagemath.org/ticket/11200#comment:21
<p>
Great, ready to be included in any 4.7.1.alpha now!
</p>
TicketjdemeyerWed, 08 Jun 2011 20:54:35 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/11200#comment:22
https://trac.sagemath.org/ticket/11200#comment:22
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-4.7.1.alpha3</em>
</li>
</ul>
Ticket