FIX: ClosureSubgroup assumes closure is in parent.

[hulpke]

Can happen in perfect subgroup computation, as reported by Serge Bouc on 4/8/19.
Added assertion to catch this sitiuation in other cases.

[ChrisJefferson]

Thanks for fixing this so quickly!

[codecov]

Codecov Report

Merging #3397 into master will decrease coverage by 7.13%.
The diff coverage is 50%.

@@            Coverage Diff             @@
##           master    #3397      +/-   ##
==========================================
- Coverage   85.16%   78.03%   -7.14%     
==========================================
  Files         697      692       -5     
  Lines      344092   340395    -3697     
==========================================
- Hits       293035   265616   -27419     
- Misses      51057    74779   +23722

Impacted Files	Coverage Δ
lib/grplatt.gi	51.4% <0%> (-22.47%)	⬇️
lib/grp.gi	79.69% <100%> (-9.55%)	⬇️
lib/ctbllatt.gi	4.94% <0%> (-79.62%)	⬇️
lib/algliess.gi	5.95% <0%> (-72%)	⬇️
lib/proto.gi	13.4% <0%> (-71.14%)	⬇️
lib/ctblpope.gi	8% <0%> (-69.47%)	⬇️
lib/contfrac.gi	31.42% <0%> (-67.15%)	⬇️
lib/lierep.gi	23.58% <0%> (-62.55%)	⬇️
lib/schursym.gi	10.6% <0%> (-58.51%)	⬇️
lib/ctblmaps.gi	18.62% <0%> (-57.86%)	⬇️
... and 230 more

[coveralls]

Coverage increased (+0.05%) to 85.207% when pulling ad32437 on hulpke:fixes into 27c845b on gap-system:master.

[olexandr-konovalov]

When merging, remember to squash and merge please.

[olexandr-konovalov]

@hulpke for the release notes, could you please give a proper description of the fix? Thanks.

[olexandr-konovalov]

@hulpke in more details, why I am having difficulties trying to describe this in release notes.

1. Different text in PR title and main commit

   ClosureSubgroup assumes closure is in parent
   ClosureSubgroup assumes closee is in parent

2. There are no changes in ClosureSubgroup - they are in ClosureSubgroupNC and LatticeViaRadical

3. Is "ClosureSubgroup assumes closure is in parent" a result of a fix, or a problem occurring in some computations?

[olexandr-konovalov]

@hulpke does this look good?

LatticeViaRadical called ClosureSubgroupNC assuming that the parent contained all generators. It now calls ClosureGroup instead, since this can not be always guaranteed (this could happen, for example, in perfect subgroup computation). Also added an assertion to ClosureSubgroupNC to catch this situation in other cases. [Reported by Serge Bouc].

[olexandr-konovalov]

I've put this version in #3503 - if you have any corrections, please leave them as comments there.