Sage: Ticket #14402: Implement tensor product of infinite crystals
https://trac.sagemath.org/ticket/14402
<p>
Currently tensor product of infinite crystals does not work well, likely due to assumptions that the crystals are finite. This implements a new tensor product of crystals class for handling infinite crystals.
</p>
<hr />
<p>
Apply:
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/14402/trac_14402-tensor_product_infinite_crystals-ts.patch" title="Attachment 'trac_14402-tensor_product_infinite_crystals-ts.patch' in Ticket #14402">trac_14402-tensor_product_infinite_crystals-ts.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/14402/trac_14402-tensor_product_infinite_crystals-ts.patch" title="Download"></a>
</li></ul>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/14402
Trac 1.1.6tscrimMon, 15 Apr 2013 18:35:11 GMTdependencies set
https://trac.sagemath.org/ticket/14402#comment:1
https://trac.sagemath.org/ticket/14402#comment:1
<ul>
<li><strong>dependencies</strong>
set to <em>#14454</em>
</li>
</ul>
Ticketbsalisbury1Fri, 19 Apr 2013 15:53:55 GMTstatus, milestone changed
https://trac.sagemath.org/ticket/14402#comment:2
https://trac.sagemath.org/ticket/14402#comment:2
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-5.9</em> to <em>sage-5.10</em>
</li>
</ul>
TicketaschillingWed, 15 May 2013 21:44:32 GMT
https://trac.sagemath.org/ticket/14402#comment:3
https://trac.sagemath.org/ticket/14402#comment:3
<p>
Hi Ben and Travis,
</p>
<p>
Thanks for your work on this! Here are some initial comments:
</p>
<ul><li>The patch needs proper header (needs to be exported).
</li></ul><ul><li>Please specify in the documentation of <a class="missing wiki">TensorProductOfCrystalsOptions?</a>=<a class="missing wiki">GlobalOptions?</a> what the various options are and what they mean. Currently this is not explained.
</li></ul><ul><li>Please remove the period at the end of line 379 of tensor_product.py
</li></ul><ul><li>It might be easier to explain the tensor product rule using the signature rule (in addition to the formulas that you use). That is more conceptual and usually easier to follow. Also, you introduce the notation a_i(k), but then you do not actually use it in the formulas for \varepsilon and \varphi. Why not?
</li></ul><ul><li>You just mention "Examples with non-regular and infinite crystals::". Perhaps say a few more words what is special about them. Also, please mention the Kashiwara and antiKashiwara options in the documentation of <a class="missing wiki">TensorProductOfCrystals?</a>, so that the user can find this and is aware of the options!
</li></ul><ul><li>The computation of epsilon and phi in your code does not seem very efficient
<pre class="wiki"> Ep = lambda k: sum(self[-j].epsilon(i) for j in range(1, k+1))
Ph = lambda k: sum(self[-j].phi(i) for j in range(1, k))
return max(Ep(k) - Ph(k) for k in range(1, len(self)+1))
</pre></li></ul><p>
You are computing the partial sums over and over. Why not something like
</p>
<pre class="wiki"> a = [ self[-1].epsilon(i) ]
for j in range(1,k):
a += [a[-1]+self[-j-1].epsilon(i) - self[-j].phi(i)]
return max(a)
</pre><p>
and similarly for phi. You could also optimize the computation of a_i(k) (which I guess is in _sig) and then use the formula of epsilon in terms of a_i(k).
</p>
<ul><li>Could you please explain why the previous implementation in <a class="missing wiki">CrystalOfWords?</a> did not do the same thing?
</li></ul><p>
So much for now.
</p>
<p>
Anne
</p>
TickettscrimFri, 17 May 2013 20:15:11 GMTdependencies, description changed
https://trac.sagemath.org/ticket/14402#comment:4
https://trac.sagemath.org/ticket/14402#comment:4
<ul>
<li><strong>dependencies</strong>
changed from <em>#14454</em> to <em>#14454 #14266</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/14402?action=diff&version=4">diff</a>)
</li>
</ul>
<p>
Hey Anne,
</p>
<p>
Here's a new version of the patch which changes the computation of <code>epsilon</code> and <code>phi</code> and caches in the parent <code>_sig</code>. I also added a note on the global option on convention to <code>TensorProductOfCrystals</code>.
</p>
<p>
I've added documentation about the signature rule, but this does not apply for non-regular crystals. For example, consider the highest weight element in B infinity tensored with itself. Both <code>phi_i</code> and <code>epsilon_i</code> are 0 for all <code>i</code>, so by the signature rule, this would be <code>0</code> for <code>f_i</code> which is not the case.
</p>
<p>
For the previous implementation, did you mean the old <code>TensorProductOfCrystalsElement</code>? If so, then it assumed the signature rule gave the crystal structure, which is why it didn't work. I didn't want to put this into the doc since it's an implementation detail, but if you think it should be, then we can add it in.
</p>
<p>
Also the dependency on <a class="closed ticket" href="https://trac.sagemath.org/ticket/14266" title="enhancement: Pretty Console Output --> ascii art (closed: fixed)">#14266</a> is trivial due to a change of <code>sources.py</code>, and this can easily be commuted past.
</p>
<p>
Thank you for doing the review,<br />
Travis
</p>
TicketaschillingThu, 30 May 2013 06:22:06 GMT
https://trac.sagemath.org/ticket/14402#comment:5
https://trac.sagemath.org/ticket/14402#comment:5
<p>
Hi Travis,
</p>
<p>
I left a review patch on the sage-combinat queue. In particular, I think the formula for \phi_i in terms of the a_i was not quite right in your patch, so I tried to correct it (it is now very simple, namely max(\lambda_i+a_i(k))). Please check that you agree! I also changed the code accordingly. The tests still pass. Since the change did not seem to make a difference for the tests, it might be a good idea to put some stronger tests in that check all possible cases for the \epsilon_i and \phi_i, so that you are sure that the code is doing what it is supposed to be doing (perhaps run some exhaustive tests for regular crystals in some example against the alternative implementation).
</p>
<p>
If you are happy with the review patch you can fold it in and make the above changes as well.
</p>
<p>
Thanks!
</p>
<p>
Anne
</p>
TickettscrimFri, 31 May 2013 01:20:04 GMTattachment set
https://trac.sagemath.org/ticket/14402
https://trac.sagemath.org/ticket/14402
<ul>
<li><strong>attachment</strong>
set to <em>trac_14402-tensor_product_infinite_crystals-ts.patch</em>
</li>
</ul>
TickettscrimFri, 31 May 2013 01:23:52 GMT
https://trac.sagemath.org/ticket/14402#comment:6
https://trac.sagemath.org/ticket/14402#comment:6
<p>
Hey Anne,
</p>
<p>
The reason why tests didn't break is because it is equivalent. To see this, note that
</p>
<pre class="wiki">a_i(k+1) = a_i(k) + \epsilon_i(b_{k+1}) - \phi_i(b_k)
</pre><p>
which is how <code>_sig()</code> is recursively computed. However this way is much more clear and clean. I've uploaded the folded patch and pushed to the queue.
</p>
<p>
Best,<br />
Travis
</p>
<p>
For patchbot:
</p>
<p>
Apply: trac_14402-tensor_product_infinite_crystals-ts.patch
</p>
TicketaschillingFri, 31 May 2013 04:45:36 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/14402#comment:7
https://trac.sagemath.org/ticket/14402#comment:7
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Anne Schilling</em>
</li>
</ul>
TicketjdemeyerFri, 31 May 2013 07:00:20 GMTdependencies, milestone changed
https://trac.sagemath.org/ticket/14402#comment:8
https://trac.sagemath.org/ticket/14402#comment:8
<ul>
<li><strong>dependencies</strong>
changed from <em>#14454 #14266</em> to <em>#14454, #14266</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-5.10</em> to <em>sage-5.11</em>
</li>
</ul>
TickettscrimFri, 31 May 2013 15:55:54 GMT
https://trac.sagemath.org/ticket/14402#comment:9
https://trac.sagemath.org/ticket/14402#comment:9
<p>
Hey Anne,
</p>
<p>
Thank you for doing the review.
</p>
<p>
Best,<br />
Travis
</p>
TicketjdemeyerThu, 06 Jun 2013 12:32:26 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/14402#comment:10
https://trac.sagemath.org/ticket/14402#comment:10
<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-5.11.beta0</em>
</li>
</ul>
Ticket