Open fingolfin opened 1 year ago
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
.
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 theirgap.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:
SOTRec.groupFromData
to directly create pc groups whenUSE_PCP = false
USE_PCP
: either...SOTGroup(2*3*5*7, 1 : UsePcp);
SOTGroup(IsPcpGroup, 2*3*5*7, 1);
; for symmetry alsoIsPcGroup
andIsPermGroup
could be supported. Note that many GAP group constructors support this, e.g. you can doSL(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 tofalse
; 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?)