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

Faster iterator for permutation groups #28653

Open
videlec opened this issue Oct 25, 2019 · 32 comments
Open

Faster iterator for permutation groups #28653

videlec opened this issue Oct 25, 2019 · 32 comments

Comments

@videlec
Copy link
Contributor

videlec commented Oct 25, 2019

The default iterator for permutation group makes use of a strong generating system (when the group is not cyclic). This is expensive and should be avoided for faster code when the group is well known or small. However, as noticed in #28539, the RecursiveEnumeratedSet generates elements in a different order in Python 2 and Python 3 and makes it delicate to use.

For some named permutation groups (symmetric, alternating, dihedral) we implement direct straightforward algorithms.

CC: @slel

Component: group theory

Author: Vincent Delecroix

Branch/Commit: public/ticket-28653 @ 78088d9

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

@videlec videlec added this to the sage-9.0 milestone Oct 25, 2019
@videlec

This comment has been minimized.

@embray
Copy link
Contributor

embray commented Jan 6, 2020

comment:2

Ticket retargeted after milestone closed

@embray embray modified the milestones: sage-9.0, sage-9.1 Jan 6, 2020
@mkoeppe
Copy link
Member

mkoeppe commented Apr 14, 2020

comment:3

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 Apr 14, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Aug 29, 2020
@slel
Copy link
Member

slel commented Aug 31, 2020

comment:5

Different order in Python 2 and Python 3 is no longer be a problem.

@fchapoton
Copy link
Contributor

comment:6

voila une premiere tentative naive. Pas bien plus rapide, en fait


New commits:

126c5d7specific iterators for SymmetricGroup and AlternatingGroup

@fchapoton
Copy link
Contributor

Commit: 126c5d7

@fchapoton
Copy link
Contributor

Branch: public/ticket-28563

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 13, 2020

Changed commit from 126c5d7 to ac936ff

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 13, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ac936ffspecific iterators for SymmetricGroup and AlternatingGroup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 13, 2020

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

2a136b4better iterator for AlternatingGroup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 13, 2020

Changed commit from ac936ff to 2a136b4

@fchapoton
Copy link
Contributor

Changed branch from public/ticket-28563 to public/ticket-28653

@fchapoton
Copy link
Contributor

comment:9

is this reasonable now ?

@videlec
Copy link
Contributor Author

videlec commented Oct 14, 2020

comment:10

What I thought was to reuse sage.combinat.permutation_cython.pyx. But in this module, it was decided to use unsigned int whereas PermutationGroupElement uses int. This is quite unfortunate.

If you use the swap algorithm from Knuth, it is trivial to make it work for A_n since permutations are alternatively even and odd (two neighboring permutations in the iterator differs by a transposition).

@fchapoton
Copy link
Contributor

comment:11

Voila une nouvelle tentative.

@fchapoton
Copy link
Contributor

Changed dependencies from #28652 to none

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 6, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e4d5636specific iterators for SymmetricGroup and AlternatingGroup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 6, 2020

Changed commit from 2a136b4 to e4d5636

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 6, 2020

Changed commit from e4d5636 to 85f9a14

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 6, 2020

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

85f9a14fix for symmetric group

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 6, 2020

Changed commit from 85f9a14 to 07c3969

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 6, 2020

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

07c3969fix doctest

@slel

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 7, 2020

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

78088d9Merge branch 'public/ticket-28653' in 9.3.b3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 7, 2020

Changed commit from 07c3969 to 78088d9

@fchapoton
Copy link
Contributor

comment:17

bon, alors evidemment, ca casse un certain nombre de doctests. Avant de se lancer dans l'etude au cas par cas de ces doctests, je voudrais savoir si le code proposé est une bonne idée ou pas.

En particulier, pour les permutations, vaut-il mieux passer par itertools comme je le fait ou bien faire la meme chose que pour les permutations alternées ?

peut-etre aussi peut-on utiliser une maniere plus efficace de créer une permutation ?

@videlec
Copy link
Contributor Author

videlec commented Dec 7, 2020

comment:18

Les permutations sont des tableaux d'entiers en C. Voir comment:10.

@fchapoton
Copy link
Contributor

comment:19

Certes. Mais tu veux dire quoi, exactement ? Qu'il faut ré-implementer l'algo de Knuth au lieu d'utiliser la version existante, qui ne se passe pas au niveau array ?

@videlec
Copy link
Contributor Author

videlec commented Dec 7, 2020

comment:20

Qu'il faut réécrire les fonctions de sage/combinat/permutation_cython.pyx de manière à ce qu'elles soient aussi utilisables par PermutationGroupElement.

@mkoeppe
Copy link
Member

mkoeppe commented Mar 15, 2021

comment:21

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Mar 15, 2021
@roed314
Copy link
Contributor

roed314 commented Apr 28, 2021

comment:22

There are multiple failing tests.

@mkoeppe
Copy link
Member

mkoeppe commented Jul 19, 2021

comment:23

Setting a new milestone for this ticket based on a cursory review.

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 2, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
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

6 participants