-
-
Notifications
You must be signed in to change notification settings - Fork 584
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
More improvements for braid groups involving permutations, and a 2-generator presentation #35985
Conversation
Change _standard_lift_Tietze for braids
Libgd reduce deps
src/sage/groups/artin.py
Outdated
OUTPUT: | ||
|
||
A permutation. | ||
An element of the Coxeter group or an element of a symmetric group. |
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.
An element of the Coxeter group or an element of a symmetric group. | |
An element of the Coxeter group ``W``. |
src/sage/groups/artin.py
Outdated
In = coxeter_matrix.index_set() | ||
for ii, i in enumerate(In): | ||
for j in In[ii+1:]: |
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.
Please ignore (IMO stupid) suggestions regarding nonessential code style that moves it further from common mathematical notation.
src/sage/groups/braid.py
Outdated
The image of ``self`` under the natural projection map to ``W`` | ||
as either a standard permutation or an element of a symmetric group. |
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.
Revert this change as the symmetric group is an example of the Coxeter group W
and this extra (unnecessary) verbosity makes it more confusing.
src/sage/groups/braid.py
Outdated
- ``isomorphism`` -- boolean (default ``False``). If ``True`` an isomorphism | ||
from ``self`` and the isomorphic group and its inverse are provided |
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.
- ``isomorphism`` -- boolean (default ``False``). If ``True`` an isomorphism | |
from ``self`` and the isomorphic group and its inverse are provided | |
- ``isomorphism`` -- boolean (default: ``False``); if ``True``, then an isomorphism | |
from ``self`` and the isomorphic group and its inverse is also returned |
src/sage/groups/braid.py
Outdated
@@ -3407,7 +3519,7 @@ def BraidGroup(n=None, names='s'): | |||
- ``n`` -- integer or ``None`` (default). The number of | |||
strands. If not specified the ``names`` are counted and the | |||
group is assumed to have one more strand than generators. | |||
|
|||
+ |
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.
Please revert this entire change.
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 do not understand this entire change. There was an extra +
due to my fingers, removed.
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.
Blanklines between inputs are unnecessary and evil IMO. I don't want to add them anywhere, especially where it wasn't before. (It also helps prevent merge conflicts as it is not where you are making changes. Many of your other changes fall under this too, but I don't disapprove of them.)
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 reverted both changes, but actually in this file for most classes there is a blank line (not introduced by me)
src/sage/groups/braid.py
Outdated
@@ -3526,6 +3638,7 @@ class group of the punctured disk. | |||
sage: A(x1^-1, s1) | |||
x1*x2^-1*x1^-1 | |||
""" | |||
|
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.
Remove; same as before.
src/sage/groups/braid.py
Outdated
through the presentation of two gens for ``self`` | ||
|
||
INPUT: | ||
|
||
- `H` -- Another group | ||
|
||
EXAMPLES:: | ||
|
||
sage: B = BraidGroup(5) | ||
sage: B.epimorphisms(SymmetricGroup(5)) | ||
[Generic morphism: | ||
From: Braid group on 5 strands | ||
To: Symmetric group of order 5! as a permutation group | ||
Defn: s0 |--> (1,5) | ||
s1 |--> (4,5) | ||
s2 |--> (3,4) | ||
s3 |--> (2,3)] |
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.
through the presentation of two gens for ``self`` | |
INPUT: | |
- `H` -- Another group | |
EXAMPLES:: | |
sage: B = BraidGroup(5) | |
sage: B.epimorphisms(SymmetricGroup(5)) | |
[Generic morphism: | |
From: Braid group on 5 strands | |
To: Symmetric group of order 5! as a permutation group | |
Defn: s0 |--> (1,5) | |
s1 |--> (4,5) | |
s2 |--> (3,4) | |
s3 |--> (2,3)] | |
through the :meth:`two generator presentation | |
<presentation_two_generators>` of ``self``. | |
INPUT: | |
- `H` -- another group | |
EXAMPLES:: | |
sage: B = BraidGroup(5) | |
sage: B.epimorphisms(SymmetricGroup(5)) | |
[Generic morphism: | |
From: Braid group on 5 strands | |
To: Symmetric group of order 5! as a permutation group | |
Defn: s0 |--> (1,5) | |
s1 |--> (4,5) | |
s2 |--> (3,4) | |
s3 |--> (2,3)] |
Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>
Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>
Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>
Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>
Now the method |
It doesn't seem like the homspace knows how to make itself using the more specific class. I would put that off for another ticket. |
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.
Just a few more details. A number of your changes are not really useful (e.g., reordering the import statements in some arbitrary order (yes, I know it is alphabetical, but that is still an arbitrary choice)) that could create trivial conflicts with other changes, but it's already done. However, the stylistic changes will not get a positive review from me.
src/sage/groups/braid.py
Outdated
sage: c1 == b.permutation() | ||
True | ||
|
||
From a permutation it is also possible to recover the permutation braid:: |
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.
From a permutation it is also possible to recover the permutation braid:: | |
It also applies the canonical section to a permutation:: |
The current version implies a certain uniqueness that is simply not true. Consider
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 put another sentence. I really want to keep the expression permutation braid
(or positive permutation braid
) since is a common concept in braid group theory. Please check it.
I applied you comments. One should be reviewed again. Probably not now, by I think that in general |
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.
Thank you. I am going to approve this PR; just make the one last change and then I will set this to a positive review.
Erase the period Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>
Thank you. |
Documentation preview for this PR (built with commit 29ca7a7; changes) is ready! 🎉 |
This PR is a continuation of #35214.
_element_constructor
._standard_lift_Tietze
has been simplified (for a permutation) and now it gives a shortest word for the lift but not necesarily the smallest for the lexicographic order. The former code has been commented.presentation2gens
provides a presentation of the braid group with two generators. The general methodepimorphism
has been adapted to braid groups using this presentation and it is much faster.📝 Checklist
⌛ Dependencies