Fix two bugs in IsomorphismPermGroupForMatrixGroup

[fingolfin]

First bug: if the input group G has a nice monomorphism which has G as source, it is always returned, even if it is not surjective.

Secondly, if G has a nice monomorphism whose range is not a perm group, then NicomorphismOfGeneralMatrixGroup is called to compute a new morphism into a permutation group. This is then also set as nice monomorphism -- but of course that only has an effect if no nice monomorphism had been set before.

But if it was set before, then we might end up restricting the range of the new map produced by NicomorphismOfGeneralMatrixGroup to the wrong group (namely NiceObject(G)).

Fixes #6205 (at least the "superficial" part of it, not yet clear why it produces a worse nice mono).

[ThomasBreuer]

Thanks.
The bug will be fixed by the proposed change.

However, the GeneralRestrictedMapping result is not as good as one expects (both with and without the change from this pull request).

gap> G:= AtlasGroup( "M11", IsMatrixGroup );;
gap> iso:= IsomorphismPermGroup( G );;
gap> HasIsBijective( iso );
true
gap> G:= AtlasGroup( "M11", IsMatrixGroup );;
gap> NiceMonomorphism( G );;
gap> iso:= IsomorphismPermGroup( G );;
gap> HasIsBijective( iso );
false

Something that is returned by IsomorphismPermGroup should know that it is bijective.
I am not sure what the best fix for this problem could be.

[ThomasBreuer]

This fix belongs to the context of issue #5355.

[ThomasBreuer]

Sorry but I do not understand the new tests.
As far as I see, they do not exhibit the bug before the change:
The Source of nice is not identical with G, and this was checked already before the change.
Here is a distinguishing example that fails in GAP 4.15.1 and passes with the current pull request.

gap> G:=Group(Z(3)^0*[[2, 2, 1],[0, 2, 2],[2, 0, 1]]);;
gap> nice:= ActionHomomorphism(G, GF(3)^3);;
gap> HasNiceMonomorphism(G);  # double check: no nice mono has been set so far
false
gap> SetNiceMonomorphism(G, nice);
gap> IsBijective(IsomorphismPermGroupForMatrixGroup(G));
true
gap> IsBijective(IsomorphismPermGroup(G));
true

[Joseph-Edwards]

I have also been looking into IsomorphismPermGroup after noticing an issue in semigroups/Semigroups#1087. I cooked up a minimal example, and it seems the changes introduced in this PR have had some interesting impacts on it.

Before this PR:

gap> G := Group([[1]]);;
gap> S5 := SymmetricGroup(5);;
gap> map := GeneralMappingByElements(G, S5, [DirectProductElement([One(G), One(S5)])]);;
gap> SetIsInjective(map, true);
gap> SetNiceMonomorphism(G, map);
gap> iso := IsomorphismPermGroup(G);
<general mapping: Group([ [ [ 1 ] ] ]) -> SymmetricGroup( [ 1 .. 5 ] ) >
gap> Size(Source(iso));
1
gap> Size(Range(iso));
120

which is wrong for reasons explained in #6205.

With the changes from this PR:

gap> G := Group([[1]]);;
gap> S5 := SymmetricGroup(5);;
gap> map := GeneralMappingByElements(G, S5, [DirectProductElement([One(G), One(S5)])]);;
gap> SetIsInjective(map, true);
gap> SetNiceMonomorphism(G, map);
gap> iso := IsomorphismPermGroup(G);
Error, recursion depth trap (5000) in
  Source( nice ) at /home/joseph/Dev/Comp_Maths/gap/lib/grpnice.gi:114 called from 
NiceObject( G ) at /home/joseph/Dev/Comp_Maths/gap/lib/grpnice.gi:1076 called from
Enumerator( elms ) at /home/joseph/Dev/Comp_Maths/gap/lib/mapping.gi:975 called from
ImagesSet( map, Source( map ) ) at /home/joseph/Dev/Comp_Maths/gap/lib/mapping.gi:998 called from
ImagesSource( nice ) at /home/joseph/Dev/Comp_Maths/gap/lib/grpnice.gi:115 called from
NiceObject( G ) at /home/joseph/Dev/Comp_Maths/gap/lib/grpnice.gi:1076 called from
...  at *stdin*:6
you may 'return;'

This might simply be user error on my part, but thought I would share it here in case this is a bug.

[ThomasBreuer]

@Joseph-Edwards Thanks for this report.
I will have a look at the problem.

[ThomasBreuer]

The explanation for the error described by @Joseph-Edwards is the same as the one in #6209:
The changes from this pull request force an IsSurjective call for map already in the IsomorphismPermGroup call.
As a consequence, ImagesSet( map, G ) is called, and we get into the infinite recursion.

[fingolfin]

@ThomasBreuer indeed, my tests are insufficient, I got carried away while modifying them back and forth between "is surjective / is not surjective" and "source is G / source is larger", and "nice object is/isn't a perm group". I've now modified it to test all combinations.

[fingolfin]

I've also add a commit which ensures that the map returned by IsomorphismPermGroupForMatrixGroup has IsBijective set to true. Many other IsomorphismBLAH methods already do that.

[ThomasBreuer]

Great. Thanks.