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

Groups: Finite presentation of permutation groups #12986

Merged
merged 7 commits into from Aug 14, 2017

Conversation

Projects
None yet
2 participants
@valglad
Contributor

valglad commented Jul 17, 2017

This PR implements the method presentation for the PermutationGroup class: it returns an FpGroup instance isomorphic to the permutation group. To assist in this, two new keyword arguments were added to coset_enumeration to allow the function to return an incomplete coset table where the function would normally exit with an error, and to be able to start coset enumeration with a partially filled table.

Additionally:

  • the keyword max_cosets was removed from some functions where it wasn't used.
  • is_subgroup method for the FreeGroup was removed too as F.is_subgroup(G) would only return True when F==G so the method is not useful.

Update: the "multi step version" commit replaces the presentation method with its multi-step version - the tests on several groups have shown that it's running time is noticeably smaller. The groups were:

  • SymmetricGroup(6)
  • PermutationGroup(Permutation(0,1,5,2)(3,7,4,6),Permutation(0,3,5,4)(1,6,2,7))
  • PermutationGroup(Permutation(0,1,5,2)(3,7,4,6),Permutation(0,3,5,4)(1,6,2,7), Permutation(0,1,5))
  • P = AlternatingGroup(5).
    The max_stack_size attribute of CosetTable was decreased to 100 as it greatly sped up the runtime of coset_enumeration_c in the presentation function (for the presentation of SymmetricGroup(6), the time was reduced from about 24 to just over 2 seconds).
Show outdated Hide outdated sympy/combinatorics/coset_table.py
###############################################################################
# COSET ENUMERATION #
###############################################################################
# relator-based method
def coset_enumeration_r(fp_grp, Y, max_cosets=None):
def coset_enumeration_r(fp_grp, Y, max_cosets=None, resume=None,

This comment has been minimized.

@jksuom

jksuom Aug 9, 2017

Member

resume refers to an unfinished table. I think that a noun would be more suitable than a verb. Maybe something like draft.

@jksuom

jksuom Aug 9, 2017

Member

resume refers to an unfinished table. I think that a noun would be more suitable than a verb. Maybe something like draft.

This comment has been minimized.

@valglad

valglad Aug 9, 2017

Contributor

That does make sense

@valglad

valglad Aug 9, 2017

Contributor

That does make sense

Show outdated Hide outdated sympy/combinatorics/perm_groups.py
return FpGroup(F, [x**order])
half_gens = G.generators[0:(len_g+1)//2]
H = PermutationGroup(half_gens)

This comment has been minimized.

@jksuom

jksuom Aug 10, 2017

Member

Should H be made the trivial subgroup when len_g is small instead of repeatedly dividing by 2?

@jksuom

jksuom Aug 10, 2017

Member

Should H be made the trivial subgroup when len_g is small instead of repeatedly dividing by 2?

This comment has been minimized.

@valglad

valglad Aug 10, 2017

Contributor

I did try it out with some examples, and there didn't seem to be any significant performance gain compared with dividing by 2. I could probably run some more tests to make sure, as last time I worked on it was several weeks ago.

@valglad

valglad Aug 10, 2017

Contributor

I did try it out with some examples, and there didn't seem to be any significant performance gain compared with dividing by 2. I could probably run some more tests to make sure, as last time I worked on it was several weeks ago.

This comment has been minimized.

@valglad

valglad Aug 11, 2017

Contributor

It is a little bit faster with half_gens = [] for G.order() < 20 so I changed it to that.

@valglad

valglad Aug 11, 2017

Contributor

It is a little bit faster with half_gens = [] for G.order() < 20 so I changed it to that.

C_p.table[0][i] = 0
gamma = 1
for alpha, x in product(range(0, n), range(2*len_g)):

This comment has been minimized.

@jksuom

jksuom Aug 10, 2017

Member

Should this be range(1, n)?

@jksuom

jksuom Aug 10, 2017

Member

Should this be range(1, n)?

This comment has been minimized.

@valglad

valglad Aug 10, 2017

Contributor

gamma = 1 could (and I think in most cases would) be first encountered in the "0-th" row of C so if alpha started with 1, this loop wouldn't go through every coset before the table runs out.

@valglad

valglad Aug 10, 2017

Contributor

gamma = 1 could (and I think in most cases would) be first encountered in the "0-th" row of C so if alpha started with 1, this loop wouldn't go through every coset before the table runs out.

@jksuom jksuom merged commit c6454a3 into sympy:master Aug 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@valglad valglad deleted the valglad:perm_present branch Aug 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment