Skip to content
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

Incorrect long element for signed permutations #25200

Closed
tscrim opened this issue Apr 18, 2018 · 12 comments
Closed

Incorrect long element for signed permutations #25200

tscrim opened this issue Apr 18, 2018 · 12 comments

Comments

@tscrim
Copy link
Collaborator

tscrim commented Apr 18, 2018

The long element is wrong:

sage: SP = SignedPermutations(5)
sage: SP.long_element()
[-5, -4, -3, -2, -1]

It should be

[-1, -2, -3, -4, -5]

CC: @sagetrac-sage-combinat @darijgr @fchapoton @stumpc5 @kevindilks

Component: combinatorics

Keywords: signed permutations

Author: Travis Scrimshaw

Branch/Commit: 7f2989d

Reviewer: Frédéric Chapoton, Jean-Philippe Labbé

Issue created by migration from https://trac.sagemath.org/ticket/25200

@tscrim tscrim added this to the sage-8.3 milestone Apr 18, 2018
@tscrim
Copy link
Collaborator Author

tscrim commented Apr 18, 2018

@tscrim
Copy link
Collaborator Author

tscrim commented Apr 18, 2018

Commit: 486febd

@tscrim
Copy link
Collaborator Author

tscrim commented Apr 18, 2018

Changed keywords from none to signed permutations

@tscrim
Copy link
Collaborator Author

tscrim commented Apr 18, 2018

comment:1

Simple fix.


New commits:

486febdFixing SignedPermutation.long_element().

@jplab
Copy link

jplab commented Apr 18, 2018

comment:2

It would be nice if the test would be against the theoretical known value.

I mean, if such a mistake was there, maybe the longest expression is not even the real longest element. After all, we know the length, so why not check against that value?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 18, 2018

Changed commit from 486febd to 7f2989d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 18, 2018

Branch pushed to git repo; I updated commit sha1. New commits:

7f2989dAdded length test against the number of positive roots.

@tscrim
Copy link
Collaborator Author

tscrim commented Apr 18, 2018

comment:4

Replying to @jplab:

It would be nice if the test would be against the theoretical known value.

I mean, if such a mistake was there, maybe the longest expression is not even the real longest element. After all, we know the length, so why not check against that value?

Good idea. I added a more systematic test.

@fchapoton
Copy link
Contributor

comment:5

ok

maybe one should add that test to finite Coxeter groups, in some later ticket

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton, Jean-Philippe Labbé

@jplab
Copy link

jplab commented Apr 18, 2018

comment:6

Nice! Thanks!

@vbraun
Copy link
Member

vbraun commented May 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants