#25706 closed defect (fixed)
Conversion map from a matrix group to its corresponding permutation group does not work
Reported by:  soehms  Owned by:  

Priority:  major  Milestone:  sage8.3 
Component:  group theory  Keywords:  days94 
Cc:  tscrim  Merged in:  
Authors:  Sebastian Oehms  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  fb4481f (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #24612  Stopgaps: 
Description
The problem is explained by the following example:
sage: G = GU(3,2); G General Unitary Group of degree 3 over Finite Field in a of size 2^2 sage: sage: MG = G.as_matrix_group(); MG Matrix group over Finite Field in a of size 2^2 with 2 generators ( [a 0 0] [a 1 1] [0 1 0] [1 1 0] [0 0 a], [1 0 0] ) sage: PG = MG.as_permutation_group(); PG Permutation Group with generators [(2,3,5)(4,7,12)(6,10,18)(8,14,21)(9,16,22)(11,20,29)(13,23,33)(17,27,26)(19,28,34)(24,25,31)(30,36,35), (1,2,4,8,15,25,12,22,32,27,7,13)(3,6,11,21,31,36,18,16,26,20,30,33)(5,9,17,14,24,23)(10,19,29,28,35,34)] sage: sage: mg = MG.an_element(); mg [a + 1 a a] [ 1 1 0] [ a 0 0] sage: conv_ori = PG.convert_map_from(MG); conv_ori Call morphism: From: Matrix group over Finite Field in a of size 2^2 with 2 generators ( [a 0 0] [a 1 1] [0 1 0] [1 1 0] [0 0 a], [1 0 0] ) To: Permutation Group with generators [(2,3,5)(4,7,12)(6,10,18)(8,14,21)(9,16,22)(11,20,29)(13,23,33)(17,27,26)(19,28,34)(24,25,31)(30,36,35), (1,2,4,8,15,25,12,22,32,27,7,13)(3,6,11,21,31,36,18,16,26,20,30,33)(5,9,17,14,24,23)(10,19,29,28,35,34)] sage: sage: img_mg = conv_ori(mg)  TypeError Traceback (most recent call last) <ipythoninput82e064fe36e82> in <module>() ............... ps/perm_gps/permgroup_element.c:4784)() 256 else: 257 if convert_dict is not None and needs_conversion: > 258 g = [tuple([convert_dict[x] for x in cycle])for cycle in g] 259 260 return g TypeError: 'sage.groups.matrix_gps.group_element.MatrixGroupElement_gap' object is not iterable
Remark: another (independent) problem is that the conversion can not be applied by instance call
Change History (15)
comment:1 Changed 3 years ago by
comment:2 Changed 3 years ago by
However, this will likely not work until these become newstyle parents: #24612.
comment:3 Changed 3 years ago by
 Branch set to u/soehms/coerce_matrixgrps_to_permutationgrps
comment:4 Changed 3 years ago by
 Commit set to 0204811fe8c63af581032f5f0c271be5d8a3ae97
 Status changed from new to needs_review
New commits:
0204811  Added _coerce_map_from_ to PermutationGroup_generic

comment:5 Changed 3 years ago by
 Commit changed from 0204811fe8c63af581032f5f0c271be5d8a3ae97 to 8f4d4d1689542c3d7bf9eb28a420ff39f8a616e0
Branch pushed to git repo; I updated commit sha1. New commits:
344907e  Moving permutation groups to the new coercion model and style parents.

1b89e95  Fixing doctests.

4d9dbdc  Merge branch 'public/coercion/perm_groups_new_parent24612' of git://trac.sagemath.org/sage into permgrps_new_coercion

9ef1ea4  Merge branch 'permgrps_new_coercion' into coerce_matrixgrps_to_permutationgrps

8f4d4d1  merged with #24612 and get it running again

comment:6 Changed 3 years ago by
 Dependencies set to #24612
 Reviewers set to Travis Scrimshaw
Positive review if a patchbot comes back green.
comment:7 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:9 Changed 3 years ago by
 Branch changed from u/soehms/coerce_matrixgrps_to_permutationgrps to u/tscrim/matrix_gp_perm_gp_coercion25706
 Commit changed from 8f4d4d1689542c3d7bf9eb28a420ff39f8a616e0 to fb4481f2f2695b9e274025f6ae7a067c116c2ee4
 Status changed from needs_work to positive_review
Trivial conflict.
New commits:
fb4481f  Merge branch 'develop' into u/tscrim/matrix_gp_perm_gp_coercion25706

comment:10 Changed 3 years ago by
Hi Travis,
thanks for fixing this! Was it my fault? Did I have a chance to detect it by myself?
This week in Saragossa had been a very nice experience to me! I learned a lot (but need to learn much more, as these questions show) and met a lot of friendly people!
Thanks again, for making this possible to me!
comment:11 Changed 3 years ago by
No, it was my fault. They were my fault on the dependency and not thinking about conflicts.
It was great to meet you in Zaragoza; I hope you will be able to contribute more!
comment:12 Changed 3 years ago by
 Branch changed from u/tscrim/matrix_gp_perm_gp_coercion25706 to fb4481f2f2695b9e274025f6ae7a067c116c2ee4
 Resolution set to fixed
 Status changed from positive_review to closed
comment:13 Changed 3 years ago by
 Commit fb4481f2f2695b9e274025f6ae7a067c116c2ee4 deleted
Hmm, why does this branch use gap
, not libgap
?
well, perhaps it's cause supposedly "converted to libgap" MatrixGroup is very far from being fully converted.
comment:14 Changed 3 years ago by
No, in fact MatrixGroup is fine, it's PermutationGroup that needs work. Indeed, instead of that conversion to strings etc one can compute the libGAP group C in as_permutation_group()
as follows:
sage: type(m) <class 'sage.groups.matrix_gps.finitely_generated.FinitelyGeneratedMatrixGroup_gap_with_category'> sage: iso=m._libgap_().IsomorphismPermGroup() sage: C=iso.Image() # of the following, if you need: sage: C=iso.Image().SmallerDegreePermutationRepresentation() sage: PG=PermutationGroup(gap_group=gap(C.Image()), canonicalize=False) # gap() is need, still
One way or another, it's much cleaner and faster, and easy to adapt to libgapconverted PermutationGroup. I've opened #26889 for this to be done.
comment:15 Changed 3 years ago by
Hi Dima,
FYI: this has been one of my first tickets. I did it with the help of Travis on the SageDays94 in July in Zaragoza. At that time I didn't know much about libgap
. In the meantime I learned a little bit more since I am trying to move the permutation groups closer to libgap
(you aleady found the correspondig ticket: #26750).
In that context I had a look at as_permutation_group
again (a couple of weeks ago), and I tried something similar as you did. I used the option algorithm
in order not to change the default behavior (I wasn't sure if there is a proper reason for that construction which leads to a different result). But unfortunately, I realized that the new attribute _permutation_group_morphism
(we had introduced in this ticket) does not keep track on optional arguments of as_permutation_group
. This made me hesitating to continue that work.
But anyway, since the latter problem obviously is a bug, I opened a ticked about that, now: #26903
By the way: Do you know why the as_permutation_group
of class FinitelyPresentedGroup
does not use IsomorphismPermGroup
?
I would implement this as a coercion that could be discovered by the corresponding permutation group. So the pseudocode would be: