stertooy commented 3 years ago

See https://github.com/gap-packages/polycyclic/issues/46#issuecomment-836857720 for a (lengthy) explanation.

While this PR should fix the issues mentioned in #46, it could probably use some polish and/or extra checks:

Perhaps there was a good reason why IsNormal had Pcp(H, U) in it? Maybe it works correctly if U is a subgroup of H?
I've simply added a large number to the filter rank in FittingSubgroup. There's probably a better solution?

codecov[bot] commented 3 years ago

Codecov Report

Merging #61 (47e4c7c) into master (f3bdcbd) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #61   +/-   ##
=======================================
  Coverage   56.01%   56.02%           
=======================================
  Files          87       87           
  Lines       13053    13045    -8     
=======================================
- Hits         7312     7308    -4     
+ Misses       5741     5737    -4

Impacted Files	Coverage Δ
gap/basic/pcpgrps.gi	`87.70% <ø> (-0.36%)`	:arrow_down:
gap/pcpgrp/fitting.gi	`61.84% <100.00%> (+0.25%)`	:arrow_up:
gap/pcpgrp/pcpattr.gi	`31.03% <0.00%> (+13.79%)`	:arrow_up:

fingolfin commented 3 years ago

Thanks!

gap-packages / polycyclic

Fix bug in IsNormal, increase rank of our FittingSubgroup method #61

Codecov Report