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

Improve speed of Christoffel word construction #8268

Closed
seblabbe opened this issue Feb 15, 2010 · 10 comments
Closed

Improve speed of Christoffel word construction #8268

seblabbe opened this issue Feb 15, 2010 · 10 comments

Comments

@seblabbe
Copy link
Contributor

This patch adds a new implementation for construction of Christoffel words using continued fraction :

sage: %timeit words.ChristoffelWord(5, 9, algorithm='linear')
625 loops, best of 3: 67.7 µs per loop
sage: %timeit words.ChristoffelWord(5, 9, algorithm='cf')
625 loops, best of 3: 309 µs per loop

For large words, it is much faster than the actual implementation.

sage: %timeit words.ChristoffelWord(500, 90001, algorithm='linear')
5 loops, best of 3: 111 ms per loop
sage: %timeit words.ChristoffelWord(500, 90001, algorithm='cf')
125 loops, best of 3: 4 ms per loop

CC: @sagetrac-abmasse

Component: combinatorics

Keywords: christoffel words

Author: Sébastien Labbé

Reviewer: Alexandre Blondin Massé

Merged: sage-4.3.4.alpha0

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

@seblabbe seblabbe added this to the sage-4.3.4 milestone Feb 15, 2010
@seblabbe seblabbe self-assigned this Feb 15, 2010
@seblabbe
Copy link
Contributor Author

comment:1

Attachment: trac_8268_speed_up_Christoffel-sl.patch.gz

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Feb 24, 2010

comment:2

I tested the improved function. It is indeed much faster, especially when the two prime numbers are big. The code makes sense, all tests passed, and I tried also other values with very big prime numbers: no problem. The doc builds fine too.

I made very minor changes (typos, punctuation, etc.). Positive review.

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Feb 24, 2010

Author: Sébastien Labbé

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Feb 24, 2010

Changed keywords from none to christoffel words

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Feb 24, 2010

Reviewer: Alexandre Blondin Massé

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Feb 24, 2010

comment:4

Sorry, forgot two small things, uploading a new patch in a few minutes.

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Feb 24, 2010

Minor change -- apply on top

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Feb 24, 2010

comment:6

Attachment: trac_8268_review-abm.patch.gz

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Mar 2, 2010

Merged: sage-4.3.4.alpha0

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Mar 2, 2010

comment:7

Merged in this order:

  1. trac_8268_speed_up_Christoffel-sl.patch
  2. trac_8268_review-abm.patch

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

1 participant