Regarding `USE_PCP` and the use of pcp groups

[fingolfin]

Uses of global flags like USE_PCP are highly problematic because they can lead to hard to debug problems and breakage; and in the worst case even to accidentally incorrect results.

Think about it: your code, which uses SOTGrps, works perfectly. Then a user reports a bug which you just can't figure out and reproduce. After hours of searching and half a dozen emails back and forth, you finally discover the cause: they set USE_PCP:=true in their gap.ini file a couple month ago, and then promptly forgot about it.

Then there is the issue of the performance penalty for first creating a pcp group and then converting it to a pc group.

So there are two things I'd suggest to consider:

1. rewrite SOTRec.groupFromData to directly create pc groups when USE_PCP = false
2. get rid of USE_PCP: either...
   1. by removing all uses of "pcp" code and dropping the dependency on polycyclic
   2. by using GAP options, e.g.: SOTGroup(2*3*5*7, 1 : UsePcp);
   3. by using GAP constructors, i.e. using a filter as an optional first argument. Example: SOTGroup(IsPcpGroup, 2*3*5*7, 1);; for symmetry also IsPcGroup and IsPermGroup could be supported. Note that many GAP group constructors support this, e.g. you can do SL(IsPermGroup,2,2);

Regarding point 2, I'd personally just ditch any use of Pcp groups and drop the dependency on polycyclic. But if you feel you want to keep this, then option ii. is much easier than iii. I can help with either.

Note that of course one could do i now and then still do ii or iii in a future release.

By the way, I'd also suggest to get rid of USE_NC resp. to at least default it to false; your presentations should always be consistent, after all. (the only reason to use the non-NC versions then is to catch bugs in your code?)

[xpan-eileen]

SOTRec.groupFromData has been rewritten and tested out, now all solvable groups are constructed directly as PcGroups, but can be set to construct PcpGroups as well. For example, SOTGroup(2*3*5*7, 1) returns the first PcGroup in the SOTGrps list of groups of order 2*3*5*7, and SOTGroup(2*3*5*7, 1, IsPcpGroup) constructs it as PcpGroup. Similarly for the AllSOTGroups function. However, currently these functions are still written using InstallGlobalFunction instead of InstallMethod + DeclareConstructor.