-
-
Notifications
You must be signed in to change notification settings - Fork 481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement tensor product of infinite crystals #14402
Comments
Dependencies: #14454 |
comment:3
Hi Ben and Travis, Thanks for your work on this! Here are some initial comments:
You are computing the partial sums over and over. Why not something like
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).
So much for now. Anne |
This comment has been minimized.
This comment has been minimized.
comment:4
Hey Anne, Here's a new version of the patch which changes the computation of 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 For the previous implementation, did you mean the old Also the dependency on #14266 is trivial due to a change of Thank you for doing the review, Travis |
comment:5
Hi Travis, 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). If you are happy with the review patch you can fold it in and make the above changes as well. Thanks! Anne |
comment:6
Hey Anne, The reason why tests didn't break is because it is equivalent. To see this, note that
which is how Best, Travis For patchbot: Apply: trac_14402-tensor_product_infinite_crystals-ts.patch |
Reviewer: Anne Schilling |
comment:9
Hey Anne, Thank you for doing the review. Best, Travis |
Merged: sage-5.11.beta0 |
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.
Apply:
Depends on #14454
Depends on #14266
CC: @sagetrac-sage-combinat @anneschilling @bsalisbury1
Component: combinatorics
Keywords: infinite crystals, tensor product
Author: Ben Salisbury, Travis Scrimshaw
Reviewer: Anne Schilling
Merged: sage-5.11.beta0
Issue created by migration from https://trac.sagemath.org/ticket/14402
The text was updated successfully, but these errors were encountered: