-
Notifications
You must be signed in to change notification settings - Fork 161
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
Make SwapMatrixColumns
and SwapMatrixRows
faster for classic matrices (lists-of-lists)
#4951
Make SwapMatrixColumns
and SwapMatrixRows
faster for classic matrices (lists-of-lists)
#4951
Conversation
390517c
to
0715dd9
Compare
The work is done and I see no harm. The idea of course is that ideally code switching to So I see no harm in merging this. Agreed @ThomasBreuer ? |
SwapMatrixColumns
and SwapMatrixRows
faster for classic matrices (lists-of-lists)
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.
No problem with the idea and with the use of the kernel operation for matrices without holes.
I do not understand the explicit error cases in FuncSWAP_MAT_ROWS
. Apparently the code deals also with holes inside the lists:
gap> l:= [ 1, 2,, 4 ];; SWAP_MAT_ROWS( l, 2, 3 ); l;
[ 1,, 2, 4 ]
(If the hole is beyond the length of the list, the result is different.)
0715dd9
to
4b5b782
Compare
This speeds up swap operations for plain list matrices a lot, and even for "real" matrix objs, as the overhead of a GAP early method is much higher than the overhead of one written in the kernel. Now whether this gain is worth the extra complexity is another question; though of course when doing heavy linear algebra, these row and column can add up. Before: gap> mat:=IdentityMat(10,Integers);; gap> for i in [1..1000000] do SwapMatrixColumns(mat,1,2); od; time; 326 gap> for i in [1..1000000] do SwapMatrixRows(mat,1,2); od; time; 90 gap> for i in [1..1000000] do tmp:=mat[1];mat[1]:=mat[2];mat[2]:=tmp; od; time; 37 gap> mat:=IdentityMat(10,GF(2));; ConvertToMatrixRep(mat);; mat; <a 10x10 matrix over GF2> gap> for i in [1..1000000] do SwapMatrixRows(mat,1,2); od; time; 146 gap> for i in [1..1000000] do tmp:=mat[1];mat[1]:=mat[2];mat[2]:=tmp; od; time; 84 After: gap> mat:=IdentityMat(10,Integers);; gap> for i in [1..1000000] do SwapMatrixColumns(mat,1,2); od; time; 153 gap> for i in [1..1000000] do SwapMatrixRows(mat,1,2); od; time; 19 gap> for i in [1..1000000] do tmp:=mat[1];mat[1]:=mat[2];mat[2]:=tmp; od; time; 45 gap> mat:=IdentityMat(10,GF(2));; ConvertToMatrixRep(mat);; mat; <a 10x10 matrix over GF2> gap> for i in [1..1000000] do SwapMatrixRows(mat,1,2); od; time; 116 gap> for i in [1..1000000] do tmp:=mat[1];mat[1]:=mat[2];mat[2]:=tmp; od; time; 83
4b5b782
to
5851f23
Compare
Revised. Now we get
and the error handling should be consistent overall. |
Yet another commit from an unfinished branch/PR that might be considered useful on its own: faster "early methods" for SwapRows/SwapCols using kernel methods. It is not tested thoroughly and I am not even sure we want to go in this direction, but we could e.g. use this as basis for discussions during the GAP Days
This speeds up swap operations for plain list matrices a lot, and
even for "real" matrix objs, as the overhead of a GAP early method
is much higher than the overhead of one written in the kernel.
Now whether this gain is worth the extra complexity is another question;
though of course when doing heavy linear algebra, these row and column
can add up.
Before:
After: