-
-
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
Iwahori-Hecke algebra with several bases #14261
Comments
Attachment: trac_14261_iwahori_hecke.patch.gz |
comment:1
Hey Brant, I've uploaded a new version of the patch which reworks the implementation to follow the Best, Travis |
Changed author from Brant Jones to Brant Jones, Travis Scrimshaw |
comment:2
I should also state that I had to upload a new version of the patch because Brant's patch did not apply on 5.9 due to the changes in the doc. Also for got this. For patchbot: Apply: trac_14261-iwahori_hecke-ts.patch |
This comment has been minimized.
This comment has been minimized.
comment:3
New version which also adds bar involution on the T_basis and some tweaks to the documentation. For patchbot: Apply: trac_14261-iwahori_hecke-ts.patch |
comment:4
A few comments:
|
comment:5
So which would you call the KL basis, C or C' (I believe calling [one of these] the KL is fairly standard)? I think the best name should be I'll also implement the ring automorphism \alpha and implement the C basis in the next version. Thanks for looking at this Andrew. |
comment:6
It's probably not necessary to put the _basis suffix. Elsewhere, we use Sym.s, Sym.p and so on and not Sym.s_basis. Cheers, |
comment:7
New version which implements both the |
Work Issues: T <-> C basis map |
comment:8
I fixed the problem and added some more doctests and added |
Changed work issues from T <-> C basis map to none |
Dependencies: #14014 |
comment:11
Rebased over #14014. For patchbot: Apply: trac_14261-iwahori_hecke-ts.patch |
comment:12
Also rebased over #13735. |
comment:13
Rebased over #14678 (trivial: some fuzz). For patchbot: Apply: trac_14261-iwahori_hecke-ts.patch |
This comment has been minimized.
This comment has been minimized.
comment:71
Thank you Brant for your initial version of this patch which contained the core functionality. Thank you Andrew for reworking the patch and your expertise. Thank you Nicolas and Darij for your help with this patch. I've uploaded the (hopefully) final folded version which removes the dead link Andrew found and fixes the typo Brant found. Thank you all for your work in this patch, Travis For patchbot: Apply: trac_14261-iwahori_hecke-ts.patch |
comment:72
Some doctest failures:
|
comment:73
Also
|
comment:74
Fixed. I've added #15257 as a dependency for (likely) fuzz. Mike (and Anne), I've cc-ed you to note the above changes to the k-Schur book's doctests. Best, Travis |
comment:75
Hi Travis, Thanks for fixing this so quickly! I have checked and the new patch fixes the problems that Jeroen found, however, I found another issue:
takes forever. I seem to remember that even Anyway, using
The longest test is on line 388 -- but this whole block is problematic because if we took out the first test then one or more of the subsequent tests would take longer as they all (should) benefit from caching. As I don't know what the official upper limit is for a long test I am not sure which ones we need to disable. Still, taking out the block on lines 383-409 would be a good start. If 30s is too long then we have a few more to worry about. To remove a test I guess that we just replace Andrew |
Changed keywords from Iwahori Hecke algebra to Iwahori Hecke algebra, days45 |
comment:78
Replying to @AndrewAtLarge:
There is certainly no such rule as far as I know. The basic rules are: normal short tests should be less than 1s. Long tests should be less than 30s, although exceptionally (if there is a good reason) up to 60s is acceptable. The benchmark system is |
comment:79
Replying to @jdemeyer:
Thanks for the correction/clarification. Travis: I've just uploaded a new version of the patch to the combinat queue which disables the long doc-tests in the block of lines 380-410 (no other changes). I don't have permission to replace your patch on trac so can you please do this and put the patch back to a positive review if you think it's OK. Andrew |
Attachment: trac_14261-iwahori_hecke-ts.patch.gz With fixed long time tests |
comment:80
Okay, I made the group smaller (
If you're okay with this, then go ahead and set it to positive review. For patchbot: Apply: trac_14261-iwahori_hecke-ts.patch |
comment:81
Replying to @tscrim:
Hi Travis, I agree, this is better than what I was proposing. Thanks! I am happy with the changes, Andrew |
Merged: sage-5.13.beta1 |
comment:83
Travis: I thought we discussed this before, but you cannot just make changes in the doctests for the book on k-Schur functions without telling Mike or myself. The file is there so that it matches with what is in the book! PS: Ok, I see that you mentioned this earlier, but unfortunately, I am not getting any more e-mails from trac due to the spam settings! |
Set up the algebra to handle multiple bases; implemented the Kazhdan--Lusztig basis.
This is a follow up to ticket #7729.
See http://wiki.sagemath.org/HeckeAlgebras for some design discussion.
Apply:
Depends on #13735
Depends on #14014
Depends on #14678
Depends on #14516
Depends on #15257
CC: @sagetrac-sage-combinat @anneschilling @AndrewAtLarge @samclearman @zabrocki
Component: combinatorics
Keywords: Iwahori Hecke algebra, days45
Author: Brant Jones, Travis Scrimshaw, Andrew Mathas
Reviewer: Andrew Mathas, Brant Jones, Travis Scrimshaw
Merged: sage-5.13.beta1
Issue created by migration from https://trac.sagemath.org/ticket/14261
The text was updated successfully, but these errors were encountered: