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

Shuffle Product #15595

Closed
sagetrac-elixyre mannequin opened this issue Dec 27, 2013 · 35 comments
Closed

Shuffle Product #15595

sagetrac-elixyre mannequin opened this issue Dec 27, 2013 · 35 comments

Comments

@sagetrac-elixyre
Copy link
Mannequin

sagetrac-elixyre mannequin commented Dec 27, 2013

Shuffle product can be define on any iterable object so I propose to define a new class that implement an efficient shuffle product on iterables.

Currently one has two shuffle product in Sage, one is defined on words and one is defined on permutations.

The first one is slow and don't allow to compute shuffle of two list and the second is fast but only defined for Permutations.

Some benchmarks:

My Shuffle:

Time if ShuffleProduct inherits just of SageObject

sage: from sage.combinat.shuffle import ShuffleProduct
sage: a = Word("abcdefghij")
sage: b = Word("klmnopqrst")
sage: %time _ = list(ShuffleProduct(a, b, Word))
CPU times: user 17 s, sys: 994 ms, total: 18 s
Wall time: 18.4 s

Time if ShuffleProduct inherits of the evil class Parent:

sage: from sage.combinat.shuffle import ShuffleProduct
sage: a = Word("abcdefghij")
sage: b = Word("klmnopqrst")
sage: %time _ = list(ShuffleProduct(a, b, Word))
CPU times: user 32.5 s, sys: 1.7 s, total: 34.2 s
Wall time: 34.5 s

with

Parent.__init__(self, category=FiniteEnumeratedSets())

Comparaison with ShuffleProduct_w1w2:

sage: from sage.combinat.words.shuffle_product import ShuffleProduct_w1w2 as Shuffle
sage: %time _ = list(Shuffle(a, b))
CPU times: user 59.66 s, sys: 0.29 s, total: 59.94 s
Wall time: 59.90 s

CC: @nthiery @mathzeta

Component: combinatorics

Keywords: days57

Author: Jean-Baptiste Priez

Branch/Commit: 18e97c1

Reviewer: Matthieu Dien, Tomer Bauer

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

@sagetrac-elixyre sagetrac-elixyre mannequin added this to the sage-6.1 milestone Dec 27, 2013
@sagetrac-elixyre
Copy link
Mannequin Author

sagetrac-elixyre mannequin commented Dec 27, 2013

Branch: u/elixyre/ticket/15595

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 27, 2013

Commit: ade0597

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 27, 2013

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

ade0597ticket 15595

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 27, 2013

Changed commit from ade0597 to 9bc07b3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 27, 2013

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

9bc07b3delete a wrong test

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 27, 2013

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

9730233fast Shuffle product compatible with iterable objects (use linear extension)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 27, 2013

Changed commit from 9bc07b3 to 9730233

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 27, 2014

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

24c6551Merge branch 'shuffle/15595' into shufflee/15595
66c0f3ftrac 15595: Shuffle more efficient + some documentation + doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 27, 2014

Changed commit from 9730233 to 66c0f3f

@sagetrac-elixyre
Copy link
Mannequin Author

sagetrac-elixyre mannequin commented Mar 27, 2014

Author: Jean-Baptiste Priez

@sagetrac-elixyre

This comment has been minimized.

@sagetrac-elixyre

This comment has been minimized.

@MatthieuDien
Copy link

Reviewer: MatthieuDien

@MatthieuDien
Copy link

Changed keywords from none to days57

@MatthieuDien
Copy link

Changed reviewer from MatthieuDien to Matthieu Dien

@MatthieuDien
Copy link

Changed branch from u/elixyre/ticket/15595 to u/MatthieuDien/ticket/15595

@MatthieuDien
Copy link

Changed commit from 66c0f3f to 0c59c31

@MatthieuDien
Copy link

New commits:

0c59c31add ome documentation

@videlec

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2014

Changed commit from 0c59c31 to b69ae60

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2014

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

b69ae60add missing '_'s

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2014

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

a396edbfix the FIXME comment

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2014

Changed commit from b69ae60 to a396edb

@mathzeta
Copy link

Changed reviewer from Matthieu Dien to Matthieu Dien, Tomer Bauer

@mathzeta
Copy link

Changed commit from a396edb to 68d2429

@mathzeta
Copy link

Changed branch from u/MatthieuDien/ticket/15595 to u/mathzeta2/ticket/15595

@mathzeta
Copy link

comment:18

I have added a "See also" comment to sage.combinat.words.shuffle_product.py that suggests to use the new implementation. We should open a ticket to replace and deprecated ShuffleProduct_w1w2.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@rwst
Copy link

rwst commented May 13, 2014

comment:20

patchbot:

+++ b/src/sage/combinat/shuffle.py

@@ -0,0 +1,438 @@

+    a_0 \cdot \left((a_n)_{n \geqslant 1} \Cup (b_m)_{m \geqslant 0}\right)
+    + b_0 \cdot \left((a_n)_{n \geqslant 0} \Cup (b_m)_{m \geqslant 1}\right)
--- a/src/sage/combinat/words/shuffle_product.py

+++ b/src/sage/combinat/words/shuffle_product.py

@@ -1,5 +1,9 @@

Non-ascii characters inserted on 2 non-empty lines
plugins.non_ascii -- 0 seconds
========== end plugins.non_ascii ==========

@MatthieuDien
Copy link

Changed branch from u/mathzeta2/ticket/15595 to u/MatthieuDien/ticket/15595

@MatthieuDien
Copy link

Changed commit from 68d2429 to ef8433d

@MatthieuDien
Copy link

comment:22

I removed the non-ascii characters and pushed it


New commits:

8bf9f4eremoved non-ascii characters
b69ae60add missing '_'s
a396edbfix the FIXME comment
ef8433dmerge

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 4, 2014

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

18e97c1fix underscores and contains

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 4, 2014

Changed commit from ef8433d to 18e97c1

@mathzeta
Copy link

mathzeta commented Jun 4, 2014

comment:25

Now the tests in the file pass. Positive review!

The work never ends. From comment [comment:18]:

We should open a ticket to replace and deprecated ShuffleProduct_w1w2.

and have shuffle of variable number of iterables, and check if the Parent/Element system can be used efficiently and many other things...

@vbraun
Copy link
Member

vbraun commented Jun 4, 2014

Changed branch from u/MatthieuDien/ticket/15595 to 18e97c1

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

5 participants