Enhance MaximalNormalSubgroups for finite solvable groups

[hungaborhorvath]

For finite solvable groups every maximal normal subgroup contains G', thus we can factor out with this.
For abelian groups, a maximal normal subgroup looks like a maximal normal subgroup in one Sylow multiplied by all other Sylows.
For an abelian p-group maximal normal subgroups are the same as maximal subgroups, hence we can factor out by the Frattini subgroup.
For elementary abelian groups one can use NormalMaximalSubgroups from grppcatr.gi. This was the main reason I put this method here rather than into grp.gi.
MaximalSubgroups could also be used, but seems to do some extra checks which are unnecessary for elementary abelian groups.

Rank of this method is raised to become RankFilter(CanComputeSize and IsFinite and IsSolvable).

Some parts probably can be generalized for polycyclic groups, but I did not check thoroughly.

Added tests, profiling says that all relevant lines are run.

Some more testing shows that for big abelian or dihedral groups the new method is significantly faster. These are not added to the tst file. In any case, for solvable groups we spare a NormalSubgroups computation.

[hungaborhorvath]

I personally do not like that MaximalNormalSubgroups returns a list rather than a set. Nevertheless, I went with this convention in the new method.

Is there any particular reason why it is chosen to be not a set but a list?

[ChrisJefferson]

Someone else may have a better answer, but I know in some cases it is because making a Set requires sorting, and some objects are very expensive to compare with '<'.

[markuspf]

@ChrisJefferson that'd be the reason: Sets are implemented as sorted duplicate-free lists, which can become really expensive for objects like subgroups of groups. Anyone volunteering to work on datastructures yet ;)

[hungaborhorvath]

#379 has a function UnionIfCanEasilySortElements, which essentially computes the union of the arguments, unless one of them has not CanEasilySortElements. I guess all of the appropriate functions corresponding to sets/lists could be rewritten, but it probably should be done in a unified way.

[markuspf]

@hungaborhorvath implementations of Set is a can of worms, which we can discuss in a separate issue, there are questions like "when are things in a set considered equal" and "how expensive is it to determine equality" involved.

[hulpke]

What `MaximalNormalSubgroups' (and similar functions) return is a duplicate free list. For all practical purposes this behaves like a mathematical set.

It is not a Set in GAP-sense, since comparison of subgroups (which we guarantee to use the < operator and to be compatible with a lex-comparison of the element sets) is comparatively expensive.
CanEasilySortArguments is a red herring here -- it is not set for sets of subgroups, I think.

What would be the benefit of making it a Set?`

[hungaborhorvath]

@hulpke Hm, I see. So Set in GAP is not the same as a set in mathematics, and there it is guaranteed that it is sorted. So it is easier to append to it, and easier to search in it. That I would call an advantage, but only if it does not take much time to sort the new element into the existing set.

So the consensus is that it should remain a list, in all situations?

[hulpke]

All similar functions return lists, not sets.

[olexandr-konovalov]

Shouldn't the argument be "MaximalNormalSubgroups.tst"?

[hungaborhorvath]

@alex-konovalov Of course it should. I copied another (WIP) tst file and modified it, obviously left the name unchanged.... Sorry.

[hungaborhorvath]

This method can be turned with not too much effort to work for some infinite groups, as well. Would it make sense to do such modifications to this PR?

[hungaborhorvath]

Ok, the new MaximalNormalSubgroups is rewritten to work for infinite (solvable) groups, as well.
If G/G' is infinite, then there are infinitely many maximal normal subgroups,
and the method errors out. If G is nonabelian, then maximal normal subgroups
of G/G' are pulled back. If G is Abelian, then first it is represented as a
pc-group, then NormalMaximalSubgroups are computed and pulled back to the
original group.

The tst file is also rewritten to test some infinite groups.

Finally, let me mention that the tst file has some commented lines. These all run within 8 seconds on my laptop if I put them in line by line. However, when I run the tst file, then they do not finish even in one minute. Anybody has any idea why that is? Thank you.

[olexandr-konovalov]

@hungaborhorvath Regarding the test, most likely it's the assertion level. Try to change it with SetAssertionLevel(2) in the command line and you will likely see the same speed as in the test.

There is a related discussion on assertion levels here: #527, #549, #564.

[hungaborhorvath]

@alex-konovalov Thank you. I have updated the test file to run in reasonable time (10 seconds on my laptop).

[fingolfin]

So you method is actually called for arbitrary groups (not just solvable ones), and you assume that you can compute AbelianInvaits, IsAbelian, IsSolvableGroup "cheaply". Hmm

[hungaborhorvath]

Well..... yes. My gut says that if for a group we cannot even compute AbelianInvariants, IsAbelian and IsSolvableGroup cheaply, then we have little chance of computing the maximal normal subgroups cheaply by any other method. But I did not verify this feeling in any way.

[hulpke]

IsomorphismGroups is potentially expensive -- Why construct a new group and not work in the old one?

[hungaborhorvath]

@hulpke NormalMaximalSubgroups needs CanEasilyComputePcgs. I would think that finite Abelian groups should get this, but currently they do not:

gap> F := FreeGroup("x", "y", "z");;
gap> x := F.1;; y := F.2;; z := F.3;;
gap> G := F/[x^(-1)*y^(-1)*x*y, x^(-1)*z^(-1)*x*z, z^(-1)*y^(-1)*z*y, (x*y)^180, (x*y^5)^168, z];
<fp group on the generators [ x, y, z ]>
gap> IsAbelian(G); time; 
true
4
gap> AbelianInvariants(G); time;
[ 3, 4, 5, 7, 9, 32 ]
4
gap> HasSize(G);
true
gap> CanEasilyComputePcgs(G); 
false
gap> NormalMaximalSubgroups(G);; time;
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 2nd choice method found for `NormalMaximalSubgroups' on 1 arguments called from
<function "HANDLE_METHOD_NOT_FOUND">( <arguments> )
 called from read-eval loop at line 24 of *stdin*
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue
brk> 

On the other hand, IsomorphismGroups at least finishes, though really slowly....

gap> Gf := AbelianGroup(IsPcGroup, AbelianInvariants(G));; time; 
4
gap> hom := IsomorphismGroups(G, Gf);; time;
80
gap> MaxGf := NormalMaximalSubgroups(Gf);; time;
24
gap> List(MaxGf, N -> PreImage(hom, N));; time;
15256

I would prefer not to use isomorphism (or do it with NiceMonomorphism, but I do not understand properly their implementations), but that would need some work on the Abelian groups that you collected in #596

[hulpke]

The whole purpose of CanEasilyComputePcgs is that a group might know to be solvable (say because a flag has been set by the user) but there still is no good method for computing a PCGS. FpGroup are exactly of this form, unless an isomorphism to a solvable group has already been constructed.

It is not a good idea to use expensive functions (such as potentially IsomorphismGroups) just because the context is not clear (and no -- this is not really what the NiceMonomorophisms are intended for).
For FpGroups there is for example MaximalAbelianQuotient which is probably as efficient as one could hope for.

I'm slightly confused by NormalMaximalSubgroups. Aren't we talking about MaximalNormalSubgroups?

[hungaborhorvath]

@hulpke NormalMaximalSubgroups computes all maximal subgroups that are actually normal. In abelian groups they are the same as MaximalNormalSubgroups, and also the same as MaximalSubgroups. The code of the latter looked to me exactly the same as NormalMaximalSubgroups except that some extra checks are done in that one (which are unnecessary in the abelian case). That is why I used NormalMaximalSubgroups, instead.

Would you prefer that I wrote a general algorithm for abelian groups using independent generators and drop the whole isomorphism route? (I did not want to do that because an algorithm exists already, but if you think that would be the proper way, I will do that.)

BTW, again I can only stress that some of these algorithms are incredibly slow for abelian groups. After having independent generators, nothing should be slow anymore, not even isomorphisms....

About MaximalAbelianQuotient: hm, looking at the code it looks like this is the one to go for all the time, and not CommutatorFactorGroup. However, I am now slightly confused about why CommutatorFactorGroup exists at all.....

[hungaborhorvath]

Replaced CommutatorFactorGroup by MaximalAbelianQuotient

[hungaborhorvath]

BTW, Is it just me or the checking part of this page is really gone?

[olexandr-konovalov]

@hungaborhorvath Since this PR now merged, could you please add to the Wiki page for the next major release the text to include in the changes manual and the release announcement?

[hungaborhorvath]

@alex-konovalov I wrote a few sentences, does it look fine?