-
-
Notifications
You must be signed in to change notification settings - Fork 619
weak order of permutations broken #27467
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
Comments
This comment has been minimized.
This comment has been minimized.
comment:2
The problem is the multiplication for a permutation seems to have the oppose effect (by default):
So we need to override |
Author: Travis Scrimshaw |
comment:3
So the only way forward that I could see to make this work is to gut out the multiplication option for New commits:
|
Commit: |
This comment has been minimized.
This comment has been minimized.
comment:5
Moving all blocker/critical issues from 8.7 to 8.8. |
comment:6
Can someone review this? |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:9
Green patchbot. |
comment:10
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:12
Replying to @fchapoton:
Added.
Fixed.
It is at least no worse than what it is currently, but I think this makes it more consistent overall. Plus it fixes a definite bug. |
comment:13
ok, then. One should suggest somewhere loudly to use |
Reviewer: Frédéric Chapoton |
comment:14
Replying to @fchapoton:
Thank you.
Also referencing I am adding Darij as a reviewer since he gave me some comments on the ticket. |
Changed reviewer from Frédéric Chapoton to Frédéric Chapoton, Darij Grinberg |
comment:15
Possibly due to #27899, I'm getting
|
comment:16
Yes, probably correlated to this replacement in #27899 : n = self.semilength()
area = self.to_area_sequence()
- from sage.groups.perm_gps.permgroup_named import SymmetricGroup
- pi = SymmetricGroup(n).one()
+ pi = Permutations(n).one()
for j in range(n):
for i in range(area[j]):
- pi = pi.apply_simple_reflection(j-i)
- return Permutation(~pi)
+ pi = pi.apply_simple_reflection(j - i)
+ return ~pi |
comment:17
I concur, and it should just be a matter of swapping the left/right convention (which I guess should just mean returning |
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:20
Sorry, I got sidetracked from this and forgot to do this last week. This is now fixed (although unfortunately we might have missed the next release cutoff...). Volker, it would be really nice if this could slip in to 8.8 since this is a relatively big bug in the combinatorics code. |
comment:21
set to positive again |
Changed branch from public/combinat/fix_weak_order_permutations-27467 to |
sagemathgh-39262: Add script for checking for old deprecations <!-- ^ 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". --> To find deprecations that can now safely removed. Example: ``` python tools/check_deprecations.py src/sage/combinat Found 9 deprecations. 100%| Deprecations over one year ago: File: src\sage\combinat\permutation.py, PR: sagemath#26810, Closed Date: 2019-01-23 File: src\sage\combinat\permutation.py, PR: sagemath#27467, Closed Date: 2019-06-12 File: src\sage\combinat\words\finite_word.py, PR: sagemath#30187, Closed Date: 2020-09-15 File: src\sage\combinat\designs\difference_family.py, PR: sagemath#35211, Closed Date: 2023-04-01 ``` ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] The title is concise and informative. - [ ] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] 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#39262 Reported by: Tobias Diez Reviewer(s): Frédéric Chapoton, Tobias Diez
From https://groups.google.com/forum/#!topic/sage-support/6kuvliWzi84:
This manifests also in creating the weak order poset:
This is a left-vs-right issue and
Permutation
multiplication being a little strange with respect to that. We also remove the multiplication argument and convention effects fromhas_*_descent
.CC: @sagetrac-sage-combinat @nthiery @fchapoton @darijgr @sagetrac-jeremy-l-martin
Component: combinatorics
Keywords: Coxeter group
Author: Travis Scrimshaw
Branch/Commit:
fa0450e
Reviewer: Frédéric Chapoton, Darij Grinberg
Issue created by migration from https://trac.sagemath.org/ticket/27467
The text was updated successfully, but these errors were encountered: