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

Multiplication by identity in tableau.py and tableau_tuple.py #14884

Closed
darijgr opened this issue Jul 12, 2013 · 7 comments
Closed

Multiplication by identity in tableau.py and tableau_tuple.py #14884

darijgr opened this issue Jul 12, 2013 · 7 comments

Comments

@darijgr
Copy link
Contributor

darijgr commented Jul 12, 2013

This is similar to #14883 in that it removes a multiplication of a permutation by the identity permutation. This time the multiplication was not useless (it was meant to coerce the permutation into a symmetric group of large enough size), but that is easier done by just concatenating lists (given that the result is only needed as a list). There is a minor speedup (577µs to 496µs on the one of the doctested examples), but the purpose of this patch is really to replace a hack by a more logical manipulation.

Travis, feel free to fold this into your patch if you wish.

Depends on #14101

CC: @tscrim @sagetrac-sage-combinat

Component: combinatorics

Keywords: combinat, tableau, tableau tuple

Author: Darij Grinberg

Reviewer: Travis Scrimshaw

Merged: sage-5.12.beta5

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

@darijgr darijgr added this to the sage-5.12 milestone Jul 12, 2013
@darijgr
Copy link
Contributor Author

darijgr commented Jul 12, 2013

Changed dependencies from 14101 to #14101

@tscrim
Copy link
Collaborator

tscrim commented Jul 23, 2013

comment:4

Hey Darij,

I believe taking the copy slice w[:] of w is unnecessary.

sage: L = [1, 2, 3]      
sage: k = L + [3, 5, 6]
sage: L
[1, 2, 3]

Could you remove this and repost?

Thanks,

Travis

@tscrim
Copy link
Collaborator

tscrim commented Jul 23, 2013

Reviewer: Travis Scrimshaw

@darijgr
Copy link
Contributor Author

darijgr commented Jul 23, 2013

corrected version

@darijgr
Copy link
Contributor Author

darijgr commented Jul 23, 2013

comment:5

Attachment: trac_14884-tableau_times_id-dg.patch.gz

Good point; fixed. Thanks for checking this.

@tscrim
Copy link
Collaborator

tscrim commented Jul 23, 2013

comment:6

Looks good to me. Thanks Darij.

@jdemeyer
Copy link

jdemeyer commented Sep 2, 2013

Merged: sage-5.12.beta5

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

3 participants