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 iterator of word morphisms #6571

Closed
seblabbe opened this issue Jul 20, 2009 · 17 comments
Closed

Improve iterator of word morphisms #6571

seblabbe opened this issue Jul 20, 2009 · 17 comments

Comments

@seblabbe
Copy link
Contributor

Right now, we can iterate over word morphisms with specific image lengths :

    Iterator over morphisms with specific image lengths::

sage: map(str, W.iter_morphisms([2, 1]))
['WordMorphism: a->aa, b->a',
 'WordMorphism: a->aa, b->b',
 'WordMorphism: a->ab, b->a',
 'WordMorphism: a->ab, b->b',
 'WordMorphism: a->ba, b->a',
 'WordMorphism: a->ba, b->b',
 'WordMorphism: a->bb, b->a',
 'WordMorphism: a->bb, b->b']
sage: map(str, W.iter_morphisms([2, 2]))
['WordMorphism: a->aa, b->aa',
 'WordMorphism: a->aa, b->ab',
 'WordMorphism: a->aa, b->ba',
 'WordMorphism: a->aa, b->bb',
 'WordMorphism: a->ab, b->aa',
 'WordMorphism: a->ab, b->ab',
 'WordMorphism: a->ab, b->ba',
 'WordMorphism: a->ab, b->bb',
 'WordMorphism: a->ba, b->aa',
 'WordMorphism: a->ba, b->ab',
 'WordMorphism: a->ba, b->ba',
 'WordMorphism: a->ba, b->bb',
 'WordMorphism: a->bb, b->aa',
 'WordMorphism: a->bb, b->ab',
 'WordMorphism: a->bb, b->ba',
 'WordMorphism: a->bb, b->bb']
sage: map(str, W.iter_morphisms([0, 0]))
['WordMorphism: a->, b->']
sage: map(str, W.iter_morphisms([0, 1]))
['WordMorphism: a->, b->a', 'WordMorphism: a->, b->b']

I want to iterate over all (non erasing) morphisms! In particular, I want the following to work :

    sage: W = Words('ab')                 
    sage: it = W.iter_morphisms()
    sage: for _ in range(10): print it.next()
    WordMorphism: a->a, b->a
    WordMorphism: a->a, b->b
    WordMorphism: a->b, b->a
    WordMorphism: a->b, b->b
    WordMorphism: a->aa, b->a
    WordMorphism: a->aa, b->b
    WordMorphism: a->ab, b->a
    WordMorphism: a->ab, b->b
    WordMorphism: a->ba, b->a
    WordMorphism: a->ba, b->b

CC: @sagetrac-sage-combinat @saliola

Component: combinatorics

Keywords: words morphisms

Author: Sébastien Labbé, Franco Saliola

Reviewer: Franco Saliola, Sébastien Labbé

Merged: Sage 4.1.2.alpha0

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

@seblabbe seblabbe added this to the sage-4.1.2 milestone Jul 20, 2009
@seblabbe seblabbe self-assigned this Jul 20, 2009
@seblabbe
Copy link
Contributor Author

comment:2

There is a bug in my patch. Some tests are apparently failing....

@seblabbe
Copy link
Contributor Author

comment:3

Failing tests were related to #5600 because the patch I first posted here was depending on compositions-cleanup-5600-nt.patch. The patch word_iter_morphism_6571-sl.patch should now apply cleanly (including doctests) on sage-4.1.1.alpha0.

@seblabbe
Copy link
Contributor Author

Attachment: word_iter_morphism_6571-sl.patch.gz

This file depends on #6519 as well as on #5600

@seblabbe
Copy link
Contributor Author

comment:6

Since #5600 should be merge soon (it has a positive review), I just uploaded a new patch that uses the benefits of #5600.

@saliola
Copy link

saliola commented Aug 24, 2009

comment:7

I made a few changes:

  1. As written, the iter_morphisms method is recursive if it is iterating through all morphisms (it calls self.iter_morphisms(composition) for all compositions). This is not necessary and it is inefficient. I changed the code to avoid doing this.

  2. Switched from Compositions to IntegerListsLex instead: the patch converted compositions spit out by Compositions to lists; so I changed it because compositions are created from the lists output by IntegerListsLex.

  3. IntegerListsLex allows one to specify min_part, so I added a min_length keyword option (so erasing morphisms can be obtained by taking min_length=0). The default is 1 (the current behaviour).

@saliola
Copy link

saliola commented Aug 24, 2009

Apply on top of word_iter_morphism_6571-sl.patch

@saliola
Copy link

saliola commented Aug 24, 2009

comment:8

Attachment: trac_6571-reviewer.patch.gz

In case it matters: I applied the patches from #5600 before word_iter_morphism_6571-sl.patch.

@seblabbe
Copy link
Contributor Author

This patch applies over the precedent two.

@seblabbe
Copy link
Contributor Author

comment:9

Attachment: trac_6571-seb-review.patch.gz

Thanks Franco for your good review. I didn't know about IntegerListsLex. Allowing erasing morphism is great as well. I just added a small patch to correct a word in a one-line comment. I am currently building the documentation...

@seblabbe
Copy link
Contributor Author

Attachment: trac_6571-ReST-bug.patch.gz

Applies over the precedent three patches.

@seblabbe
Copy link
Contributor Author

comment:10

There was a bug in the ReST documentation. I just added trac_6571-ReST-bug.patch which corrects it. Documentation now builds without warnings. Doctests passed on words.py. I am giving a positive review to Franco's changes.

@saliola
Copy link

saliola commented Aug 25, 2009

comment:11

Sébastien's documentation changes are good.

@seblabbe
Copy link
Contributor Author

Reviewer: Franco Saliola

@seblabbe
Copy link
Contributor Author

Changed author from Sébastien Labbé to Sébastien Labbé, Franco Saliola

@seblabbe
Copy link
Contributor Author

Changed reviewer from Franco Saliola to Franco Saliola, Sébastien Labbé

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Aug 26, 2009

Merged: Sage 4.1.2.alpha0

@sagetrac-mvngu sagetrac-mvngu mannequin closed this as completed Aug 26, 2009
@fchapoton

This comment has been minimized.

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

4 participants