Added AddMatrixLeft, MultMatrixLeft functions as well as their "right" counterparts#5980
Added AddMatrixLeft, MultMatrixLeft functions as well as their "right" counterparts#5980OldAnchovyTopping wants to merge 4 commits intogap-system:masterfrom
AddMatrixLeft, MultMatrixLeft functions as well as their "right" counterparts#5980Conversation
OldAnchovyTopping
changed the title
AddMatrixLeft, MultMatrixLeft functions as well as their "right" counterparts
fingolfin
left a comment
fingolfin
left a comment
There was a problem hiding this comment.
Thanks! Some quick comments
| InstallMethod( AddMatrix, "for a mutable 8bit matrix and an 8bit matrix", | ||
| [ Is8BitMatrixRep and IsMutable, Is8BitMatrixRep ], | ||
| function( dstmat, srcmat ) | ||
| local i, j; |
There was a problem hiding this comment.
The linter is unhappy, this is one of the changes needed to make it happy:
| local i, j; | |
| local i; |
| # Assert(0, NrRows(dstmat) = NrRows(srcmat)); | ||
| # Assert(0, NrCols(dstmat) = NrCols(srcmat)); |
There was a problem hiding this comment.
These should either be removed, or turned into working code.
| # Assert(0, NrRows(dstmat) = NrRows(srcmat)); | |
| # Assert(0, NrCols(dstmat) = NrCols(srcmat)); |
or
| # Assert(0, NrRows(dstmat) = NrRows(srcmat)); | |
| # Assert(0, NrCols(dstmat) = NrCols(srcmat)); | |
| Assert(1, DimensionsMat(dstmat) = DimensionsMat(srcmat)); |
| # Assert(0, NrRows(dstmat) = NrRows(srcmat)); | ||
| # Assert(0, NrCols(dstmat) = NrCols(srcmat)); |
There was a problem hiding this comment.
| # Assert(0, NrRows(dstmat) = NrRows(srcmat)); | |
| # Assert(0, NrCols(dstmat) = NrCols(srcmat)); | |
| Assert(1, DimensionsMat(dstmat) = DimensionsMat(srcmat)); |
There was a problem hiding this comment.
Similar in other places which I did not mark
| InstallMethod( AddMatrix, "for a mutable 8bit matrix, an 8bit matrix, and a scalar", | ||
| [ Is8BitMatrixRep and IsMutable, Is8BitMatrixRep, IsFFE ], | ||
| function( dstmat, srcmat, scalar ) | ||
| local i, j; |
There was a problem hiding this comment.
The linter is unhappy, this is one of the changes needed to make it happy:
| local i, j; | |
| local i; |
| ## <Returns>nothing</Returns> | ||
| ## | ||
| ## <Description> | ||
| ## <P/> |
There was a problem hiding this comment.
No need to start with a paragraph break here, I think?
| ## <P/> |
|
|
||
| ############################################################################ | ||
| ## | ||
| ## <#GAPDoc Label="AddMatrixRight"> |
There was a problem hiding this comment.
This chunk needs to be included by one of the doc/ref/*.xml files, via <#Include Label="AddMatrixRight">
Otherwise it won't appear in the manual.
There was a problem hiding this comment.
Also it should be changed to AddMatrix :-) also below
|
|
||
| ############################################################################ | ||
| ## | ||
| ## <#GAPDoc Label="MultMatrix"> |
There was a problem hiding this comment.
This chunk also needs to be added
|
@OldAnchovyTopping did you see the comments I left you? Just wondering, no pressure -- if you can work some more on this that would be great, but of course you need to know if and when you have time for it. |
2 participants
To make progress on issue #5952 I implemented the operations of summing and multiplying matrices in-place.
Further work
These functions should now be ready to be used in
meatauto.gito speed up those computations (as well as in other other place that might benefit from in-place matrix operations).The functions also do NOT have coverage tests, that should be added sometime later too.