Opened 8 years ago
Closed 8 years ago
#14910 closed enhancement (fixed)
as_finitely_presented_group method for permutation groups
Reported by:  dshurbert  Owned by:  

Priority:  minor  Milestone:  sage5.12 
Component:  group theory  Keywords:  group presentations 
Cc:  rbeezer, vbraun, mmarco  Merged in:  sage5.12.beta3 
Authors:  Davis Shurbert  Reviewers:  Volker Braun 
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  #14790  Stopgaps: 
Description (last modified by )
The permutation group method to_presentation wraps the GAP function IsomorphismFpGroupByGenerators? which yields an isomorphism from a given group to an isomorphic finite presentation. to_presentation takes the image of the isomorphism returned by this function and wraps it as a Sage finitely presented group.
sage: G = DihedralGroup(8); G Dihedral group of order 16 as a permutation group sage: G.to_presentation() Finitely presented group < a, b  b^2, b*a^1*b*a^1, a^8 >
Depends on #14790 which gives the python generator object needed for variable names.
Apply
Attachments (1)
Change History (16)
comment:1 Changed 8 years ago by
 Description modified (diff)
comment:2 Changed 8 years ago by
comment:3 Changed 8 years ago by
 Description modified (diff)
comment:4 Changed 8 years ago by
I don't like the whole conversion to string and then evaluating it. You should convert the generators to Tietze words which are just tuples of integers and hence don't depend on the generator names.
For consistency with the other group theory code, I'd rather call the method as_finitely_presented_group()
. For example, we also have
sage: G.<a,b> = FreeGroup() sage: H = G / (a^2, b^3, a*b*~a*~b) sage: H.as_permutation_group() Permutation Group with generators [(1,2)(3,5)(4,6), (1,3,4)(2,5,6)]
Small nitpicks:
 Empty line after the title ("Return a finitely presented group...")
 Two dashes after input:
 ``reduced`` `
, this is the ReST code for en dash
comment:5 Changed 8 years ago by
Just made suggested changes, using the Tietze word of each relation was definitely a better way to approach the problem. I am currently waiting on 5.11.beta3 to finish building on my machine so I can test the current output against the new printing. The same goes for #14791.
comment:6 Changed 8 years ago by
Comments:
 Volker's Tietze word suggestion is a good one. Capitalize Tietze where you use it, isn't a person's name?

to_
oras_
? Seems Sage uses "to" more often, but you do see "as" in group theory code.
 Drop # in text of TESTS in doc tests and make this actual text that will appear properly in documentation.
 When you illustrate "reduced" in doctest, write
(reduced=True)
to make it clearer what you are illustrating.
 Drop
sage_eval
import. Check othrs
 In code. I'm sympathetic to aligning equals sign in four assignments, but I would drop it.
comment:7 Changed 8 years ago by
Made requested changes. I left the method name as as_
because it fits with the fpgroup.as_permutation_group
style. I rebased the patch to 5.11.beta3 and fixed errors arising from the new printing of group presentations from GAP.
comment:8 Changed 8 years ago by
 Status changed from new to needs_review
Ready for review?
Its better to break overly long lines, e.g.
ret_rls = tuple([F(rel_word.TietzeWordAbstractWord(image_gens).sage()) for rel_word in image_fp.RelatorsOfFpGroup()])
instead of
ret_rls = tuple([F(rel_word.TietzeWordAbstractWord(image_gens).sage()) for rel_word in image_fp.RelatorsOfFpGroup()])
comment:9 Changed 8 years ago by
Just shortened overly long line. Ready for review.
comment:10 Changed 8 years ago by
 Reviewers set to Volker Braun
 Status changed from needs_review to positive_review
 Summary changed from to_presentation method for permutation groups to as_finitely_presented_group method for permutation groups
Looks good to me
comment:11 Changed 8 years ago by
 Milestone set to sage5.12
comment:12 Changed 8 years ago by
 Status changed from positive_review to needs_work
The patch needs a proper commit message.
comment:13 Changed 8 years ago by
There is a conflict with #14791: both patches add the same reference [TWGT]
.
comment:14 Changed 8 years ago by
 Status changed from needs_work to needs_review
Fixed reference conflict, added proper commit message.
comment:15 Changed 8 years ago by
 Merged in set to sage5.12.beta3
 Resolution set to fixed
 Status changed from needs_review to closed
Variable names for free group generators are manually changed from an "f.1" and "f.2" style to an "a" and "b" style. I use
sage_eval
and a dictionary binding old generator names carried over from GAP to lexicographical generator names. If anyone has a better way to do this renaming process please don't hesitate to comment.