Conversion map from a matrix group to its corresponding permutation group does not work

[soehms]

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)
<ipython-input-8-2e064fe36e82> 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

[tscrim]

I would implement this as a coercion that could be discovered by the corresponding permutation group. So the pseudocode would be:

def _coerce_map_from_(self, G):
    try:
        PG = G.as_permutation_group()
        if PG == self:                                         
            return the_morphism_construced_from_GAP
        if self.has_coerce_map_from(PG):
            return self.coerce_map_from(PG) * the_morphism_to_PG
    except (AttributeError, TypeError):
        pass
    return super(PermutationGroup, self)._coerce_map_from_(G) 

[tscrim]

However, this will likely not work until these become new-style parents: #24612.

[soehms]

New commits:

​0204811

Added _coerce_map_from_ to PermutationGroup_generic

[git]

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_parent-24612' 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

[tscrim]

Positive review if a patchbot comes back green.

[vbraun]

Merge conflict

[tscrim]

Trivial conflict.

---

New commits:

​fb4481f

Merge branch 'develop' into u/tscrim/matrix_gp_perm_gp_coercion-25706

[soehms]

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!

[tscrim]

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!

[dimpase]

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.

[dimpase]

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 libgap-converted PermutationGroup. I've opened #26889 for this to be done.

[soehms]

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?