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

Speeding up tableau's cells_containing and weight methods #15269

Closed
darijgr opened this issue Oct 10, 2013 · 6 comments
Closed

Speeding up tableau's cells_containing and weight methods #15269

darijgr opened this issue Oct 10, 2013 · 6 comments

Comments

@darijgr
Copy link
Contributor

darijgr commented Oct 10, 2013

I know that the current implementation of tableaux in Sage is not long for this world (mutability WTF), but here are two small pieces of simple code that can be improved:
Before:

sage: %timeit Tableau([[1,1,2,8,9],[2,5,6,11],[3,7,7,13],[4,8,9],[5],[13],[14]]).cells_containing(4)
1000 loops, best of 3: 449 us per loop

After:

sage: %timeit Tableau([[1,1,2,8,9],[2,5,6,11],[3,7,7,13],[4,8,9],[5],[13],[14]]).cells_containing(4)
10000 loops, best of 3: 81.5 us per loop

Before:

sage: %timeit Tableau([[1,1,2,8,9],[2,5,6,11],[3,7,7,13],[4,8,9],[5],[13],[14]]).weight()
10000 loops, best of 3: 190 us per loop

After:

sage: %timeit Tableau([[1,1,2,8,9],[2,5,6,11],[3,7,7,13],[4,8,9],[5],[13],[14]]).weight2()
10000 loops, best of 3: 87.4 us per loop

Apply:

CC: @tscrim @sagetrac-sage-combinat @anneschilling

Component: combinatorics

Keywords: tableau, combinat

Author: Darij Grinberg

Reviewer: Travis Scrimshaw

Merged: sage-5.13.beta1

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

@darijgr darijgr added this to the sage-5.13 milestone Oct 10, 2013
@darijgr
Copy link
Contributor Author

darijgr commented Oct 10, 2013

Attachment: trac_15269-tableau-speedup-dg.patch.gz

@tscrim

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Oct 13, 2013

comment:2

Attachment: trac_15269-review-ts.patch.gz

Hey Darij,

Here's a small review patch which checks the old implementation agrees with the new one and some micro-optimizations where I was able to squeeze out a little bit more speed (on the order of 1%, but it's there). If you're happy with my changes, then go ahead and set a positive review.

Best,

Travis

@tscrim
Copy link
Collaborator

tscrim commented Oct 13, 2013

Reviewer: Travis Scrimshaw

@darijgr
Copy link
Contributor Author

darijgr commented Oct 13, 2013

comment:3

Thanks for the review! Yeah, nice fixes. I didn't know one could write max(spam for x in eggs) and get a maximum of the iterator.

for the patchbot:

apply trac_15269-tableau-speedup-dg.patch​ trac_15269-review-ts.patch​

@jdemeyer
Copy link

Merged: sage-5.13.beta1

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