Skip to content
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

Various speed improvements to polynomial factorization and the GAP meataxe #720

Merged
merged 7 commits into from
Apr 13, 2016

Conversation

fingolfin
Copy link
Member

While investigating the recog package together with Alice Niemeyer and Frank Lübeck last week, I came up with some tweaks to GAP that improved the recognition speed for some examples considerably.

First example: The following went from about 5.5 seconds to about 2.1 seconds on my laptop:

mat := IdentityMat( 300, GF(2) );;
mat{[1,2]}{[1,2]} := [[0,1],[1,0]]*Z(2);;
ConvertToMatrixRep(mat);;
M:=GModuleByMats([mat],GF(2));;
MTX.BasesCompositionSeries(M);; time;

Second example: The following went from about 78 milliseconds to 10ms. Over larger fields, it is not quite as dramatic, but still noticable (e.g. over GF(3), it went from 55ms to 13ms, and over GF(17) from 62ms to 39ms).

There are more optimization opportunities in the meataxe code. For example, it compute characteristic polynomials, then factors them. But the code for computing characteristic polynomials actually does so by first computing factors, then multiplying them... The obvious idea then is to provide a function which directly returns those factors (they won't be irreducible in general, but certainly still easier to factorize than the full char poly). Indeed, I noticed that the cvec package defines a method for that: FactorsOfCharacteristicPolynomial. Perhaps something like this should be added to GAP itself?

This affects computation of minimal and characteristic polynomials
For compressed matrices, this is much more efficient, and it should
never be slower.
Combined with the preceding commit, these give a substantial speed
up for certain meataxe examples.
factoring (x-1)^293 over GF(p) for small p is about 1.5 times faster;
over GF(2) even about 2 times faster
We avoid multiplying operations that amount to no-ops, but still
require GAP to do work. This in turn speeds up factoring polynomials
like (x-1)^293 over small fields (over GF(2) it becomes 3 times faster).
The old code first used QuotRemLaurpols to perform a trial division;
if the remainder turned out to be zero, it computed the quotient again.
Instead, we simply use QuotRemLaurpols to compute quotient and remainder
in one go.
@markuspf
Copy link
Member

These changes all look correct to me.

The only point I'd raise is that a potential speed gain from ConvertToMatrixRep should (maybe) be investigated in view of HPC-GAP (where ConvertToMatrixRep is deprecated).
This is of course not blocking this PR from being merged.

@hulpke
Copy link
Contributor

hulpke commented Apr 11, 2016

Very Commendable. If ConvertToMatrixRep is depreciated (how do we this then?) there will be enough other problems that this one should not matter.

@frankluebeck
Copy link
Member

This looks all fine to me.

@markuspf markuspf merged commit 03e2122 into gap-system:master Apr 13, 2016
@fingolfin fingolfin deleted the mh/speed branch August 18, 2016 15:34
@olexandr-konovalov olexandr-konovalov added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: added PRs introducing changes that have since been mentioned in the release notes labels Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants