-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes in Unitary and Orthogonal from Summerschool 2019 by Cheryl an… #223
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #223 +/- ##
==========================================
+ Coverage 77.87% 78.69% +0.81%
==========================================
Files 43 37 -6
Lines 18398 17205 -1193
==========================================
- Hits 14328 13539 -789
+ Misses 4070 3666 -404
|
Sporadic test failures due to TestSporadic sigh @ssiccha I am tempted to say we should just disable all of those for now until your replacement code lands (i.e. that PR should add them back) |
gap/matrix/classical.gi
Outdated
## 2.July.2019: There is a mistake in the paper here | ||
## There are maximal subgroups of Omega+(8,5) that contain | ||
## elements of order 7,13,3 | ||
if not HasElementsMultipleOf( recognise.orders, [7,13,3,312]) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So where does the 312 come from? It's not a prime, and I just had a look at POmega(8,5) and based on a random sampling it only contains elements of order 156, not 312.
Wouldn't 31 make more sense?
return fail; | ||
fi; | ||
|
||
pgrp := ProjectiveActionOnFullSpace( grp, recognise.field, d ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my laptop I get:
gap> ProjectiveActionOnFullSpace( Omega(+1,8,5), GF(5), 8);; time;
648
So this takes more than half a second. That seems rather slow. We also may end up re-computing this action later on... As it is, it caused a severe slowdown of our test suite, because now testing with the input O(+1,8,5)
is very slow. Before this PR:
gap> ReadPackage("recog", "tst/naming.g");
gap> TestNaming("SO", +1, 8, 5); time;
645
With this PR:
gap> ReadPackage("recog", "tst/naming.g");
gap> TestNaming("SO", +1, 8, 5); time;
212476
So it goes in total from 0.6 seconds to 3.5 minutes!!!
Do we really need the whole orbit? Perhaps we can instead just check the length of the orbit of a single vector or so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that we do the same computation in other cases too. So is this slower because the dimension is bigger or do we maybe not test the other cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other cases are indeed significantly smaller; just computing the orbit takes some time. Moreover 5^7 > 2^16 so "big" permutations have to be used; 4*5^7 = 320kb per permutation kills most caches quickly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aniemeyer and I are looking at this.
I'll use magma to compute the maximal subgroups of GO+(8,5). Then we can see how to distinguish them from the Omega+(8,5).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we should write down which theorem this code is based upon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some thoughts:
- It should be possible to compute only one orbit. I've tried it and unfortunately that also takes half a second.
- I haven't figured out yet how to compute the maximal sugroups of GO+(8,5) in Magma. I can get the maximal ones of Omega+(8,5), but that's it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know by now what the relevant "bad" maximal subgroup is?
Also, I noticed that while I complained that pgrp := ProjectiveActionOnFullSpace(...)
takes half a second, the call Size(pgrp)
later actually takes 5 seconds ... So yeah, we need to work on this :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gap> orb:=Orbit(G,One(G)[1], OnLines);; time;
90
gap> Length(orb);
19656
gap> pg:=Action(G,orb,OnLines);; time;
223
gap> Size(pg2); time;
8911539000000000000
1369
So perhaps this would work:
orb:=Orbit(grp, One(grp)[1], OnLines);
if not Length(orb) in [ 19656, 39000 ] then ... return false; fi
pgrp:=Action(G,orb,OnLines);
if Size(pgrp) ...
Of course pgrp
may underestimate the actual size of the group. In order to understand whether the above is sufficient or not, I'd need to better understand what the group is we want to distinguish. @aniemeyer just told me that it is O(7,5) which is of course muuuch smaller; it has size 914004000000000. So perhaps it suffices to test whether grp
resp. pgrp
has more than 914004000000000 elements; for this, one could abort the random schreier sims early as soon as one has a group bigger than that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fingolfin Oh, interesting! Aborting the schreier sims computation if we found enough elements then seems like the way to go. Cheryl and Alice had a chat about how to do this case last week (I think) and we'll try to put it into code after the GAP days.
6c51c6c
to
79f19cb
Compare
I added some "work in progress" code how we are fixing Omega+(8,5). We're not finished yet though. |
Here is our Hack.md. I haven't updated it for some time: |
Made during Summerschool 2019 by Cheryl and Alice
Rebased onto master. I think I'll go ahead and compare the gap and magma code for the classical groups on a case by case basis and, unless they differ substantially, just take over the magma version. Since the magma code is fairly well tested, that might be good enough for us. |
…d Alice