-
Notifications
You must be signed in to change notification settings - Fork 178
Make meataxe endomorphism ring computation faster in some cases #5959
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
Conversation
... at least in some examples. The performance of the previous code
also fluctuated extremely based on the RNG seed.
Before:
gap> G:=SmallGroup(24, 3);;
gap> p:=NextPrimeInt(100);;
gap> MTX.HomogeneousComponents(RegularModule(G, GF(p))[2]);; time;
3998
gap> zz := MTX.HomogeneousComponents(RegularModule(G, GF(p))[2]);;
Error, Unable to ascertain module decomposition within time limits.
...
gap> zz := MTX.HomogeneousComponents(RegularModule(G, GF(p))[2]);; time;
1061
gap> zz := MTX.HomogeneousComponents(RegularModule(G, GF(p))[2]);; time;
16724
After:
gap> G:=SmallGroup(24, 3);;
gap> p:=NextPrimeInt(100);;
gap> MTX.HomogeneousComponents(RegularModule(G, GF(p))[2]);; time;
100
gap> MTX.HomogeneousComponents(RegularModule(G, GF(p))[2]);; time;
69
gap> MTX.HomogeneousComponents(RegularModule(G, GF(p))[2]);; time;
70
gap> MTX.HomogeneousComponents(RegularModule(G, GF(p))[2]);; time;
73
hulpke
left a comment
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.
This is fine, as far as I can see. The code is a GAP4 translation I did about 15-20 years ago of code of GAP3 code written by Michael Smith. The purpose was to enable automorphism group computations, and it seemed good enough for that purpose. The state-of-the-art for this problem has gone far beyond -- Charles Leedham-Green and Eamonn O'Brien should be able to point to better solutions.
ThomasBreuer
left a comment
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.
looks good, thanks
|
We are trying to implement some state of the art meataxe functionality, but unfortunately it seems a lot of that is unpublished. We'll definitely try to use our meeting in Oberwolfach to try and get more information about all this. In the meantime, I will add a bunch of further "local" improvements as we find and implement them. E.g. I just updated this PR to not compute the full minimal polynomial (which gets expensive for larger matrices) but instead just compute a single order polynomial which usually is enough. |
|
Actually I think I'll better just merge this for now and then will open more PRs with the rest when I get to it. |
... at least in some examples. The performance of the previous code also fluctuated extremely based on the RNG seed.
Before:
After:
What I have not done yet is to try systematically with other examples if they got faster or slower (!). I think it would make sense for us to setup some benchmark set -- perhaps similar to what I did in #5958, or perhaps just a bunch of modules we then use for testing.
There is also more work to be done: the original example by @fieker that lead me to look into this was the regular module of
SmallGroup(240, 3)(not 24!) which is much larger, and which still performs badly. For this much more work is needed; one of the things missing there is applying the same "evaluate at a factor of the minpoly" trick, but there computing the minpoly gets to expensive. So we should probably add a new helper function which takes a matrix and computes a factor of the minpoly by spinning (and then optionally tries to factor it -- in my tests, factoring was basically never the bottleneck).Even with such an improvement we run in more problems. I am going to try to see how far we can push this code, but in the end we maybe have to look into completely reimplementing this in order to catch up with Magma.