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

have libgap permutation to sage use permutation instead of permutation group element #25609

Closed
stumpc5 opened this issue Jun 19, 2018 · 23 comments

Comments

@stumpc5
Copy link
Contributor

stumpc5 commented Jun 19, 2018

This is a side improvement of #25477, as seen there in comment 62 (please properly link if possible):

sage: P = PermutationGroup([(1,2),(1,2,3,4,5,6,7,8)])
sage: l = [8,7,6,5,4,3,2,1]
sage: %timeit P(l, check=False)
The slowest run took 7.58 times longer than the fastest. This could mean that an intermediate result is being cached.
10000 loops, best of 3: 44.3 µs per loop
sage: %timeit Permutation(l, check_input=False)
The slowest run took 7.85 times longer than the fastest. This could mean that an intermediate result is being cached.
100000 loops, best of 3: 6.74 µs per loop

So creating a new permutation is much faster than creating a permutation group element. If we know which permutation group we live in, there is even a faster way:

sage: one = P.one()
sage: %timeit one._generate_new(l)
The slowest run took 128.70 times longer than the fastest. This could mean that an intermediate result is being cached.
1000000 loops, best of 3: 241 ns per loop

Depends on #25477

CC: @tscrim @dimpase

Component: group theory

Keywords: permutation group, libgap

Author: Christian Stump

Branch: 2396cf0

Reviewer: Travis Scrimshaw

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

@stumpc5 stumpc5 added this to the sage-8.3 milestone Jun 19, 2018
@stumpc5
Copy link
Contributor Author

stumpc5 commented Jun 19, 2018

Dependencies: #25477

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jun 19, 2018

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 18, 2018

Commit: 3b540a1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 18, 2018

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

3b540a1merge

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2018

Changed commit from 3b540a1 to a5d0e7f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2018

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

55d5c64implemented breadth and depth first search iteration
9504bc2fixed more doctests
97ce629more doctests
b586d0cadded examples of perm gp elts into documentation
79c25c9Merge branch 'u/stumpc5/iteration_permutation_groups-25477' of git://trac.sagemath.org/sage into public/group_theory/improve_iterator_perm_gps-25477
ca2d1b3Fixing doctests due to bad rebase and some doc tweaks.
1b00bf2merged
dddbcf3Fixing trivial doctest failures.
1a3a69aFix the ordering of one more test.
a5d0e7fmerged

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2018

Changed commit from a5d0e7f to dd429d5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2018

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

dd429d5fixed last todo

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2018

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

18f93edadded sentence to docstring

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2018

Changed commit from dd429d5 to 18f93ed

@stumpc5
Copy link
Contributor Author

stumpc5 commented Sep 19, 2018

comment:7

This is not really depending on #25477, I think. But anyways, this is only a very small change...

@tscrim
Copy link
Collaborator

tscrim commented Sep 21, 2018

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Sep 21, 2018

comment:8

LGTM (but I agree that #25477 should not really be a dependency).

@vbraun
Copy link
Member

vbraun commented Oct 2, 2018

comment:9

See patchbot

@tscrim
Copy link
Collaborator

tscrim commented Oct 3, 2018

Changed commit from 18f93ed to 2396cf0

@tscrim
Copy link
Collaborator

tscrim commented Oct 3, 2018

comment:10

This fixes the errors on the patchbot (the polyhedron one was fixed in #25477 and I cannot reproduce it now here).


New commits:

1a85742Using libgap.Group(self.gens()) to avoid input string limits via libgap(self).
b52770eMerge branch 'public/group_theory/improve_iterator_perm_gps-25477' of git://trac.sagemath.org/sage into public/group_theory/improve_iterator_perm_gps-25477
916cea6Fixing doctests due to change in output order.
ca255bbMerge branch 'develop' into public/group_theory/improve_iterator_perm_gps-25477
0a25f1aMerge branch 'u/stumpc5/have_libgap_permutation_to_sage_use_permutation_instead_of_permutation_group_element' of git://trac.sagemath.org/sage into u/tscrim/libgap_to_sage_permutation_construction-25609
2396cf0Fixing doctests and making refinement_generic.pyx know the parent.

@tscrim
Copy link
Collaborator

tscrim commented Oct 3, 2018

comment:11

Green patchbot (morally).

@tscrim
Copy link
Collaborator

tscrim commented Oct 23, 2018

comment:12

Green patchbot (still).

@tscrim tscrim modified the milestones: sage-8.3, sage-8.5 Oct 23, 2018
@stumpc5
Copy link
Contributor Author

stumpc5 commented Oct 23, 2018

comment:13

Can I put it to positive review?

@tscrim
Copy link
Collaborator

tscrim commented Oct 23, 2018

comment:14

Yes. In fact, I needed someone else to review my changes. :)

@vbraun
Copy link
Member

vbraun commented Oct 24, 2018

@fchapoton
Copy link
Contributor

Changed commit from 2396cf0 to none

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