gap-packages / ClassicalMaximals

Maximal subgroups of classical groups
Other
0 stars 8 forks source link

Orthogonal C7 #114

Closed TristanPfersdorff closed 2 years ago

TristanPfersdorff commented 2 years ago

See comment below for lack of test coverage. Tests for gap/ClassicalMaximals.gi are up next in #115.

codecov[bot] commented 2 years ago

Codecov Report

Merging #114 (80bb76d) into main (da9e8b3) will decrease coverage by 1.15%. The diff coverage is 62.12%.

@@            Coverage Diff             @@
##             main     #114      +/-   ##
==========================================
- Coverage   88.75%   87.59%   -1.16%     
==========================================
  Files          14       14              
  Lines        4446     4692     +246     
==========================================
+ Hits         3946     4110     +164     
- Misses        500      582      +82     
Impacted Files Coverage Δ
gap/ClassicalMaximals.gi 58.88% <1.04%> (-4.67%) :arrow_down:
gap/TensorInducedMatrixGroups.gi 97.64% <97.02%> (-0.81%) :arrow_down:
fingolfin commented 2 years ago

When you say "too large to cover with tests" what do you mean? That you cannot even create the group because it is too slow? Or that running group recognition is too slow? In the former case, let's try to make it faster otherwise it is useless. In the latter case: simply don't try group recognition (for now), but we can still perform the other tests (e.g. is the form preserved, are the determinants right etc.)

TristanPfersdorff commented 2 years ago

When you say "too large to cover with tests" what do you mean? That you cannot even create the group because it is too slow? Or that running group recognition is too slow? In the former case, let's try to make it faster otherwise it is useless. In the latter case: simply don't try group recognition (for now), but we can still perform the other tests (e.g. is the form preserved, are the determinants right etc.)

I hadn't thought about this very thoroughly and I'm glad you brought it up, because I've run some tests concerning OrthogonalEvenTensorInducedDecompositionStabilizerInOmega and the results are very interesting. Test times are on my machine, a decent laptop:

Fortunately, the tests both pass. I'm assuming we don't just want to throw in a 4-minute test in our CI, but we could if we really wanted to. Using functions/properties like IsHyperbolicForm, IsParabolicForm and IsEllipticForm as well as IsDegenerateForm from the forms package instead of WittIndex also does not help because all of these functions (including WittIndex) are more or less just user interfaces to BaseChangeOrthogonalBilinear and BaseChangeOrthogonalQuadratic, which must be the culprits. I'm assuming that conjugating the group runs into similar functions internally, which would explain why constructing the group and checking for membership in Omega takes about the same time. There isn't much we can do about this other than to not tests whether the form is preserved and not use RECOG for, say, n > 64 or maybe n > 81. Even then this test would still take almost 2 minutes and I'm not really seeing any other solutions (apart from not conjugating our groups at all which also sucks), so nothing seems to satisfy.