-
-
Notifications
You must be signed in to change notification settings - Fork 517
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
Implementation of the general Burau representation for Artin groups #39415
base: develop
Are you sure you want to change the base?
Conversation
179fdcb
to
de91281
Compare
Documentation preview for this PR (built with commit 02a7986; changes) is ready! 🎉 |
I have started to look at it. A small comment first, there is an unused |
Thanks for taking a look. As you can see in the doctest in |
src/sage/groups/artin.py
Outdated
@@ -156,6 +164,132 @@ def coxeter_group_element(self, W=None): | |||
In = W.index_set() | |||
return W.prod(s[In[abs(i)-1]] for i in self.Tietze()) | |||
|
|||
def burau_matrix(self, var='t', reduced=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think reduced is not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
OUTPUT: | ||
|
||
The Burau matrix of the Artin group element over the Laurent | ||
polynomial ring in the variable ``var``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe here it is worth to recall that for type A one gets unitary representation for braid groups.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a comment about this in the main definition part.
|
||
if var == 't': | ||
return ret | ||
from sage.rings.polynomial.laurent_polynomial_ring import LaurentPolynomialRing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to import it in the beginning of the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to import it in the beginning of the file?
I think local imports should be preferred as they perform better with regard to our modularization goals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, this is why I made them local. It's a bit of a judgement call I feel.
LGTM |
@enriqueartal Thank you. Can you approve the PR and then set the positive review label? |
gens = [one - MS([SparseEntry(i, j, val(coxeter_matrix[index_set[i], index_set[j]])) | ||
for j in range(n)]) | ||
for i in range(n)] | ||
invs = [one - q**-2 * MS([SparseEntry(i, j, val(coxeter_matrix[index_set[i], index_set[j]])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mathematically, there is no difference, but it would be more consistent with the citations from the literature if you swapped the indices in val(coxeter_matrix...)
. But if you don't see a need for that, that's fine with me.
…for Artin groups <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> We provide an implementation of the generalized Burau representation (following [this paper by Bapat and Queffelec](https://arxiv.org/abs/2409.00144)). We also allow Artin groups for arbitrary type to be constructed. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39415 Reported by: Travis Scrimshaw Reviewer(s): Enrique Manuel Artal Bartolo, Sebastian Oehms, Travis Scrimshaw
We provide an implementation of the generalized Burau representation (following this paper by Bapat and Queffelec). We also allow Artin groups for arbitrary type to be constructed.
📝 Checklist
⌛ Dependencies