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 equality tests for words #8187

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

improve equality tests for words #8187

seblabbe opened this issue Feb 5, 2010 · 15 comments

Comments

@seblabbe
Copy link
Contributor

seblabbe commented Feb 5, 2010

More often, when we compare two words, we test their equality and not that one is less than the other. So why not implement the equatlity test!? This ticket does this for datatypes and for math objects.

  1. This patch adds equality test for WordDatatype_str, WordDatatype_list and WordDatatype_tuple (via __richcmp__) usefull when comparing two words represented by the same kind of data. It is now as fast as the python object :
BEFORE:

    sage: w = Word(range(10000))
    sage: z = Word(range(10000))
    sage: %timeit w == z
    125 loops, best of 3: 4.08 ms per loop

AFTER:

    sage: w = Word(range(10000))
    sage: z = Word(range(10000))
    sage: %timeit w == z
    625 loops, best of 3: 422 µs per loop

PYTHON OBJECT:

    sage: w = range(10000)
    sage: z = range(10000)
    sage: %timeit w == z
    625 loops, best of 3: 442 µs per loop
BEFORE:

    sage: w = Word(tuple(range(10000)))
    sage: z = Word(tuple(range(10000)))
    sage: %timeit w == z
    125 loops, best of 3: 3.97 ms per loop

AFTER:

    sage: w = Word(tuple(range(10000)))
    sage: z = Word(tuple(range(10000)))
    sage: %timeit w == z
    625 loops, best of 3: 419 µs per loop

PYTHON OBJECT:

    sage: w = tuple(range(10000))
    sage: z = tuple(range(10000))
    sage: %timeit w == z
    625 loops, best of 3: 420 µs per loop
BEFORE:

    sage: w = Word('a'*10000)
    sage: z = Word('a'*10000)
    sage: %timeit w == z
    125 loops, best of 3: 3.9 ms per loop

AFTER:

    sage: w = Word('a'*10000)
    sage: z = Word('a'*10000)
    sage: %timeit w == z
    625 loops, best of 3: 2.36 µs per loop

PYTHON OBJECT:

    sage: w = 'a'*10000
    sage: z = 'a'*10000
    sage: %timeit w == z
    625 loops, best of 3: 2.03 µs per loop

  1. Add the __eq__ and __ne__ for Word_class because it is faster than using __cmp__ (especially when parent alphabet are defined). These functions are used to test the equality of two words represented by different python objects (datatypes).

no parents :

BEFORE:

    sage: L = range(10000)
    sage: t = tuple(L)
    sage: w = Word(L)
    sage: z = Word(t)
    sage: type(w)
    <class 'sage.combinat.words.word.FiniteWord_list'>
    sage: type(z)
    <class 'sage.combinat.words.word.FiniteWord_tuple'>
    sage: %timeit w == z
    125 loops, best of 3: 3.69 ms per loop


AFTER:

    sage: L = range(10000)
    sage: t = tuple(L)
    sage: w = Word(L)
    sage: z = Word(t)
    sage: type(w)
    <class 'sage.combinat.words.word.FiniteWord_list'>
    sage: type(z)
    <class 'sage.combinat.words.word.FiniteWord_tuple'>
    sage: %timeit w == z
    625 loops, best of 3: 1.44 ms per loop

with parents (!!):

BEFORE:

    sage: W = Words([0,1,2])
    sage: w = W([0, 1, 1, 2]*4000)
    sage: z = W([0, 1, 1, 2]*4000)
    sage: %timeit w == z
    5 loops, best of 3: 63 ms per loop

AFTER:

    sage: W = Words([0,1,2])
    sage: w = W([0, 1, 1, 2]*4000)
    sage: z = W([0, 1, 1, 2]*4000)
    sage: %timeit w == z
    125 loops, best of 3: 2.57 ms per loop

CC: @sagetrac-abmasse @videlec @saliola

Component: combinatorics

Keywords: words

Work Issues: equality

Author: Sébastien Labbé

Reviewer: Alexandre Blondin Massé

Merged: sage-4.3.4.alpha0

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

@seblabbe

This comment has been minimized.

@seblabbe

This comment has been minimized.

@seblabbe
Copy link
Contributor Author

seblabbe commented Feb 5, 2010

Changed keywords from none to words

@seblabbe
Copy link
Contributor Author

seblabbe commented Feb 5, 2010

Work Issues: equality

@seblabbe
Copy link
Contributor Author

comment:4

This patch seems to introduce some problems. See

http://groups.google.com/group/sage-combinat-devel/browse_thread/thread/9e90bbeb0328034c

Needs work...

@seblabbe

This comment has been minimized.

@seblabbe
Copy link
Contributor Author

Attachment: trac_8187_equality_words-sl.patch.gz

@seblabbe
Copy link
Contributor Author

comment:6

This patch seems to introduce some problems.

Finally, the problem were already present. This ticket was simply making them appear! The problem is being tracked at #8232.

I just uploaded a new patch.

Needs review again!

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Feb 24, 2010

comment:7

I tested the patch and everything seems fine. Although it modifies several basic functions, it doesn't seem to have any side effect. I couldn't check all documentation since some part of the code is written in Cython, but the functions I did check built correctly.

I've made minor changes in the doc: just formatting some part of the code or correcting typos. If Sébastien agrees with my change, I allow him to set the path to positive review.

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Feb 24, 2010

Doc, formatting and typos changes -- apply on top of the main patch

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Feb 24, 2010

comment:8

Attachment: trac_8187_review-abm.patch.gz

I forgot to mention that equality tests are indeed improved in some cases, and I haven't encountered any worse case, which is good !

@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

Reviewer: Alexandre Blondin Massé

@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:11

Merged in this order:

  1. trac_8187_equality_words-sl.patch
  2. trac_8187_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