Skip to content

Added AddMatrixLeft, MultMatrixLeft functions as well as their "right" counterparts #5980

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions lib/listcoef.gd
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,13 @@ DeclareOperation(
## <ManSection>
## <Oper Name="MultVector" Arg='list1, mul'/>
## <Oper Name="MultVectorLeft" Arg='list1, mul'/>
## <Oper Name="MultVectorRight" Arg='list1, mul'/>
## <Returns>nothing</Returns>
##
## <Description>
## This operation calculates <A>mul</A>*<A>list1</A> in-place.
## These operations multiply the entries of <A>list1</A> by <A>mul</A> in-place.
## The operation <Ref Oper="MultVectorLeft"/> calculates <A>mul \cdot list1</A> and
## the operation <Ref Oper="MultVectorRight"/> calculates <A>list1 \cdot mul</A>.
## <P/>
## Note that <C>MultVector</C> is just a synonym for
## <C>MultVectorLeft</C>.
Expand All @@ -122,7 +125,10 @@ DeclareOperation(
"MultVectorLeft",
[ IsMutable and IsList,
IsObject ] );
# For VectorObj objects there also exists a MultVectorRight operation
DeclareOperation(
"MultVectorRight",
[ IsMutable and IsList,
IsObject ] );
DeclareSynonym( "MultVector", MultVectorLeft );

#############################################################################
Expand Down
45 changes: 45 additions & 0 deletions lib/listcoef.gi
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,51 @@
MULT_VECTOR_VECFFES );


#############################################################################
##
#M MultVectorRight( <list>, <mul> )
##
InstallMethod( MultVectorRight,
"for a mutable dense list, and an object",
[ IsDenseList and IsMutable,
IsObject ],
function( l, m )
local i;
for i in [ 1 .. Length(l) ] do
l[i] := l[i] * m;
od;

Check warning on line 319 in lib/listcoef.gi

View check run for this annotation

Codecov / codecov/patch

lib/listcoef.gi#L317-L319

Added lines #L317 - L319 were not covered by tests
end );
InstallOtherMethod( MultVectorRight, "error if immutable",
[ IsList, IsObject ],
L1_IMMUTABLE_ERROR);

# TODO: Check if MULT_VECTOR_RIGHT_2 exists and if not, implement it
# InstallMethod( MultVectorRight,
# "kernel method for a mutable dense small list, and an object",
# IsCollsElms,
# [ IsSmallList and IsDenseList and IsMutable,
# IsObject ],
# MULT_VECTOR_LEFT_2
# );

InstallMethod( MultVectorRight,
"kernel method for a mutable dense plain list of \
cyclotomics, and a cyclotomic",
IsCollsElms,
[ IsDenseList and IsMutable and IsPlistRep and IsCyclotomicCollection,
IsCyclotomic ],
MULT_VECTOR_2_FAST
);
# TODO: Check if MULT_VECTOR_VECFFES does the right thing for multiplying form the right
# InstallMethod( MultVectorRight,
# "kernel method for a mutable row vector of ffes in \
# plain list rep, and an ffe",
# IsCollsElms,
# [ IsRowVector and IsMutable and IsPlistRep and IsFFECollection,
# IsFFE],0,
# MULT_VECTOR_VECFFES );


#############################################################################
##
#M RightShiftRowVector( <list>, <shift>, <fill> )
Expand Down
142 changes: 142 additions & 0 deletions lib/matobj.gi
Original file line number Diff line number Diff line change
Expand Up @@ -1877,9 +1877,151 @@

end );

#############################################################################
##
#M AddMatrix( <mat1>, <mat2> )
##

InstallMethod( AddMatrix, "for a mutable matrix object and a matrix object",
[ IsMatrixOrMatrixObj and IsMutable, IsMatrixOrMatrixObj ],
function( dstmat, srcmat )
local i, j;
# Assert(0, NrRows(dstmat) = NrRows(srcmat));
# Assert(0, NrCols(dstmat) = NrCols(srcmat));
for i in [1..NrRows(dstmat)] do
for j in [1..NrCols(dstmat)] do
dstmat[i,j] := dstmat[i,j] + srcmat[i,j];
od;
od;

Check warning on line 1895 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1891-L1895

Added lines #L1891 - L1895 were not covered by tests
end );

InstallEarlyMethod( AddMatrix,
function( dstmat, srcmat )
local i;
if IsPlistRep(dstmat) and IsPlistRep(srcmat) then

Check warning on line 1901 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1901

Added line #L1901 was not covered by tests
# Assert(0, NrRows(dstmat) = NrRows(srcmat));
# Assert(0, NrCols(dstmat) = NrCols(srcmat));
for i in [1..NrRows(dstmat)] do
AddRowVector(dstmat[i], srcmat[i]);
od;

Check warning on line 1906 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1904-L1906

Added lines #L1904 - L1906 were not covered by tests
else
TryNextMethod();
fi;

Check warning on line 1909 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1908-L1909

Added lines #L1908 - L1909 were not covered by tests
end );

InstallMethod( AddMatrix, "for a mutable 8bit matrix and an 8bit matrix",
[ Is8BitMatrixRep and IsMutable, Is8BitMatrixRep ],
function( dstmat, srcmat )
local i, j;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linter is unhappy, this is one of the changes needed to make it happy:

Suggested change
local i, j;
local i;

# Assert(0, NrRows(dstmat) = NrRows(srcmat));
# Assert(0, NrCols(dstmat) = NrCols(srcmat));
Comment on lines +1916 to +1917
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should either be removed, or turned into working code.

Suggested change
# Assert(0, NrRows(dstmat) = NrRows(srcmat));
# Assert(0, NrCols(dstmat) = NrCols(srcmat));

or

Suggested change
# Assert(0, NrRows(dstmat) = NrRows(srcmat));
# Assert(0, NrCols(dstmat) = NrCols(srcmat));
Assert(1, DimensionsMat(dstmat) = DimensionsMat(srcmat));

for i in [1..NrRows(dstmat)] do
ADD_COEFFS_VEC8BIT_3(dstmat[i], srcmat[i]);
od;

Check warning on line 1920 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1918-L1920

Added lines #L1918 - L1920 were not covered by tests
end );

#############################################################################
##
#M AddMatrix( <mat1>, <mat2>, <mult> )
##

InstallMethod( AddMatrix, "for a mutable matrix object, a matrix object, and a scalar",
[ IsMatrixOrMatrixObj and IsMutable, IsMatrixOrMatrixObj, IsScalar ],
function( dstmat, srcmat, scalar )
local i, j;
# Assert(0, NrRows(dstmat) = NrRows(srcmat));
# Assert(0, NrCols(dstmat) = NrCols(srcmat));
Comment on lines +1932 to +1933
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Assert(0, NrRows(dstmat) = NrRows(srcmat));
# Assert(0, NrCols(dstmat) = NrCols(srcmat));
Assert(1, DimensionsMat(dstmat) = DimensionsMat(srcmat));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar in other places which I did not mark

for i in [1..NrRows(dstmat)] do
for j in [1..NrCols(dstmat)] do
dstmat[i,j] := dstmat[i,j] + srcmat[i,j] * scalar;
od;
od;

Check warning on line 1938 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1934-L1938

Added lines #L1934 - L1938 were not covered by tests
end );

InstallEarlyMethod( AddMatrix,
function( dstmat, srcmat, scalar )
local i;
if IsPlistRep(dstmat) and IsPlistRep(srcmat) then

Check warning on line 1944 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1944

Added line #L1944 was not covered by tests
# Assert(0, NrRows(dstmat) = NrRows(srcmat));
# Assert(0, NrCols(dstmat) = NrCols(srcmat));
for i in [1..NrRows(dstmat)] do
AddRowVector(dstmat[i], srcmat[i], scalar);
od;

Check warning on line 1949 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1947-L1949

Added lines #L1947 - L1949 were not covered by tests
else
TryNextMethod();
fi;

Check warning on line 1952 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1951-L1952

Added lines #L1951 - L1952 were not covered by tests
end );

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linter is unhappy, this is one of the changes needed to make it happy:

Suggested change
local i, j;
local i;

# Assert(0, NrRows(dstmat) = NrRows(srcmat));
# Assert(0, NrCols(dstmat) = NrCols(srcmat));
for i in [1..NrRows(dstmat)] do
ADD_COEFFS_VEC8BIT_3(dstmat[i], srcmat[i], scalar);
od;

Check warning on line 1963 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1961-L1963

Added lines #L1961 - L1963 were not covered by tests
end );

#############################################################################
##
#M MultMatrixRight( <mat>, <mult> )
##

InstallMethod( MultMatrixRight, "for a mutable matrix object and a scalar",
[ IsMatrixOrMatrixObj and IsMutable, IsScalar ],
function( mat, scalar )
local i, j;
for i in [1..NrRows(mat)] do
for j in [1..NrCols(mat)] do
mat[i,j] := mat[i,j] * scalar;
od;
od;

Check warning on line 1979 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1975-L1979

Added lines #L1975 - L1979 were not covered by tests
end );

InstallEarlyMethod( MultMatrixRight,
function( mat, scalar )
local i;
if IsPlistRep(mat) and IsScalar(scalar) then
for i in [1..NrRows(mat)] do
MultVectorRight(mat[i], scalar);
od;

Check warning on line 1988 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1985-L1988

Added lines #L1985 - L1988 were not covered by tests
else
TryNextMethod();
fi;

Check warning on line 1991 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L1990-L1991

Added lines #L1990 - L1991 were not covered by tests
end );

#############################################################################
##
#M MultMatrixLeft( <mat>, <mult> )
##

InstallMethod( MultMatrixLeft, "for a mutable matrix object and a scalar",
[ IsMatrixOrMatrixObj and IsMutable, IsScalar ],
function( mat, scalar )
local i, j;
for i in [1..NrRows(mat)] do
for j in [1..NrCols(mat)] do
mat[i,j] := scalar * mat[i,j];
od;
od;

Check warning on line 2007 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L2003-L2007

Added lines #L2003 - L2007 were not covered by tests
end );

InstallEarlyMethod( MultMatrixLeft,
function( mat, scalar )
local i;
if IsPlistRep(mat) and IsScalar(scalar) then
for i in [1..NrRows(mat)] do
MultVectorLeft(mat[i], scalar);
od;

Check warning on line 2016 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L2013-L2016

Added lines #L2013 - L2016 were not covered by tests
else
TryNextMethod();
fi;

Check warning on line 2019 in lib/matobj.gi

View check run for this annotation

Codecov / codecov/patch

lib/matobj.gi#L2018-L2019

Added lines #L2018 - L2019 were not covered by tests
end );

############################################################################
## Fallback method for DeterminantMatrix

InstallMethod(DeterminantMatrix, ["IsMatrixObj"],
function( mat )
return DeterminantMat( Unpack( mat ) );
Expand Down
83 changes: 83 additions & 0 deletions lib/matobj2.gd
Original file line number Diff line number Diff line change
Expand Up @@ -2052,3 +2052,86 @@ DeclareOperationKernel( "SwapMatrixRows", [ IsMatrixOrMatrixObj and IsMutable, I
## <#/GAPDoc>
##
DeclareOperationKernel( "SwapMatrixColumns", [ IsMatrixOrMatrixObj and IsMutable, IsInt, IsInt ], SWAP_MAT_COLS );

############################################################################
##
## <#GAPDoc Label="AddMatrixRight">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it should be changed to AddMatrix :-) also below

## <ManSection>
## <Oper Name="AddMatrixRight" Arg='M, N[, c]'/>
##
## <Returns>nothing</Returns>
##
## <Description>
## <P/>
## Computes the calculation <M>M + c \cdot N</M> in-place, storing the result in <A>M</A>.
## If both of the matrices are lists-of-lists, then the program utilises <Ref Oper="AddRowVector"/>
## to perform the operation even faster.
## <Example><![CDATA[
## gap> mat1 := [ [ 1, 2 ], [ 3, 4 ] ];
## [ [ 1, 2 ], [ 3, 4 ] ]
## gap> mat2 := [ [ 1, 0 ], [ 3, -1 ] ];
## [ [ 1, 0 ], [ 3, -1 ] ]
## gap> AddMatrix( mat1, mat2, 2 );
## gap> mat1;
## [ [ 3, 2 ], [ 9, 2 ] ]
## gap> mat2;
## [ [ 1, 0 ], [ 3, -1 ] ]
## gap> AddMatrix( mat1, [ [ 1, 0], [ 3, -1] ] );
## gap> mat1;
## [ [ 4, 2 ], [ 12, 1 ] ]
## ]]></Example>
## </Description>
## </ManSection>
## <#/GAPDoc>
##
DeclareOperation( "AddMatrix", [ IsMatrixOrMatrixObj and IsMutable, IsMatrixOrMatrixObj ] );
DeclareOperation( "AddMatrix", [ IsMatrixOrMatrixObj and IsMutable, IsMatrixOrMatrixObj, IsScalar ] );

############################################################################
##
## <#GAPDoc Label="MultMatrix">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This chunk also needs to be added

## <ManSection>
## <Oper Name="MultMatrix" Arg='mat, c'/>
## <Oper Name="MultMatrixLeft" Arg='mat, c'/>
## <Oper Name="MultMatrixRight" Arg='mat, c'/>
##
## <Returns>nothing</Returns>
##
## <Description>
## <P/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to start with a paragraph break here, I think?

Suggested change
## <P/>

## These functions multiply the entries of <A>mat</A> by <A>c</A> in-place.
## <Ref Oper="MultMatrixRight"/> performs the operation <M>mat \cdot c</M>,
## whereas <Ref Oper="MultMatrixLeft"/> performs the operation <M>c \cdot mat</M>
## and <Ref Oper="MultMatrix"/> is an alias for <Ref Oper="MultMatrixLeft"/>.
## In all of these, if the matrix <A>mat</A> is a lists-of-lists, then the program
## utilises the fast in-place operations <Ref Oper="MultVectorRight"/> and <Ref Oper="MultVectorLeft"/>.
## to perform the operation even faster.
## <Example><![CDATA[
## gap> mat1 := [ [ 1, 2 ], [ 3, 4 ] ];
## [ [ 1, 2 ], [ 3, 4 ] ]
## gap> MultMatrixRight(mat1, -2);
## gap> mat1;
## [ [ -2, -4 ], [ -6, -8 ] ]
## gap> MultMatrix(mat1, -2); # Note that this is the same as calling MultMatrixLeft(mat1, -2)
## gap> mat1;
## [ [ 4, 8 ], [ 12, 16 ] ]
## gap> A := FreeAssociativeAlgebra(Rationals, 2);
## <algebra over Rationals, with 2 generators>
## gap> mat2 := [ [ A.1, A.2 ], [ A.1 * 2, A.2 * 3 ] ];
## [ [ (1)*x.1, (1)*x.2 ], [ (2)*x.1, (3)*x.2 ] ]
## gap> MultMatrixLeft(mat2, A.1);
## gap> mat2;
## [ [ (1)*x.1^2, (1)*x.1*x.2 ], [ (2)*x.1^2, (3)*x.1*x.2 ] ]
## gap> mat2 := [ [ A.1, A.2 ], [ A.1 * 2, A.2 * 3 ] ];
## [ [ (1)*x.1, (1)*x.2 ], [ (2)*x.1, (3)*x.2 ] ]
## gap> MultMatrixRight(mat2, A.1);
## gap> mat2;
## [ [ (1)*x.1^2, (1)*x.2*x.1 ], [ (2)*x.1^2, (3)*x.2*x.1 ] ]
## ]]></Example>
## </Description>
## </ManSection>
## <#/GAPDoc>
##
DeclareOperation( "MultMatrixRight", [ IsMatrixOrMatrixObj and IsMutable, IsScalar ] );
DeclareOperation( "MultMatrixLeft", [ IsMatrixOrMatrixObj and IsMutable, IsScalar ] );
DeclareSynonym( "MultMatrix", MultMatrixLeft);
Loading