HasAbelianFactorGroup, HasSolvableFactorGroup, etc., does not require N to be normal subgroup

[hungaborhorvath]

I was browsing grp.gi while came across these functions: HasAbelianFactorGroup, HasSolvableFactorGroup, HasElementaryAbelianFactorGroup. These all do what one would think to do (and what is written in the manual), except that N is not required to be a normal subgroup, and is not checked for it. In particular, weird results are obtained if N is not a normal subgroup, but just some subset:

gap> G := SymmetricGroup(4);
Sym( [ 1 .. 4 ] )
gap> HasAbelianFactorGroup(G, [(1,2)]);
true
gap> G=NormalClosure(G, Group((1,2)));
true

This for example can be interpreted as checked whether the factor by the normal subgroup generated by (1,2) is abelian. This interpretation, though, is not always right:

gap> HasAbelianFactorGroup(G, Group([(1,2,3)]));
true
gap> HasAbelianFactorGroup(G, [(1,2,3)]);
false
gap> NormalClosure(G, Group((1,2,3)))=AlternatingGroup(4);
true

I believe that either the manual should say that undefined results will happen if the user does not make sure that N is a normal subgroup, or the current methods should be the NC versions, and the non-NC versions should check whether N is really a normal subgroup. Maybe a subgroup check is enough, and then the results is about factor by the normal closure.

Another thing I noticed about HasSolvableFactorGroup (there is a typo in line 4540: abelian should really be solvable), is that it checks if any element of the derived series is in N. Would it not be enough to check for the last element of the derived series? Or, alternatively, computing the elements one by one (not using the derived series) and checking each whether they are already in N (which could spare some computation on the further commutators)? This is of course not really an issue for finite permutation groups, but rather for some fp-groups where derived subgroup computation and checking subsets could be expensive.

[hulpke]

I will amend the documentation (and add a level 2 assertion).

[hungaborhorvath]

@hulpke Thank you.

Do you know why all elements of the derived series are checked instead of only the last one?

[hungaborhorvath]

Completely fixed by #1066