Skip to content

Add IsGenericMatrixRep matrix object type #6322

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.

Sign up for GitHub

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

Open
fingolfin wants to merge 23 commits into
base: master
Choose a base branch
from
Open

Add IsGenericMatrixRep matrix object type #6322

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
ab613e3
Add IsFlatPlistMatrixRep matrix object type
fingolfin Apr 12, 2026
c7ce377
hook up documentation
fingolfin Apr 14, 2026
cd615d8
Fix gaplint warnings
fingolfin Apr 14, 2026
20cb5d9
Make tst/testspecial/method-not-found-where.g more robust
fingolfin Apr 14, 2026
bfeea74
address some review comments
fingolfin Apr 14, 2026
4c8e15a
Remove CopyFlatPlistMatrixRow
fingolfin Apr 14, 2026
ec0d2fe
Rename IsFlatPlistMatrixRep to IsGenericMatrixRep
fingolfin Apr 16, 2026
ce80685
Rename files matobjflatplist -> matobjgeneric
fingolfin Apr 16, 2026
d3850ea
Inline InverseRowsForFlatPlistMatrix
fingolfin Apr 17, 2026
ec8d4ea
MakeIsGenericMatrixRep requires plist
fingolfin Apr 17, 2026
779cd77
Tighten MakeIsGenericMatrixRep
fingolfin Apr 17, 2026
9b59c1a
Inverse should error, not return fail
fingolfin Apr 17, 2026
9021a26
Generalize vec*mat and mat*vec
fingolfin Apr 17, 2026
3b20fa9
More tests
fingolfin Apr 17, 2026
8a2fe0c
Unify matobj family code
fingolfin Apr 17, 2026
33779c5
More tests
fingolfin Apr 17, 2026
0cac33b
Simplify code
fingolfin Apr 17, 2026
7f2e2a4
Yet more filter/family code simplification
fingolfin Apr 17, 2026
f246b18
more test coverage
fingolfin Apr 17, 2026
a7ea309
oops
fingolfin Apr 17, 2026
6abd94f
cleanup
fingolfin Apr 17, 2026
1ba59c4
some tweaks
fingolfin Apr 20, 2026
a2ea205
revert a family change
fingolfin Apr 20, 2026
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
18 changes: 12 additions & 6 deletions benchmark/matobj/bench-matelm-access.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,23 @@ RunMatTest := function(desc, m)
TestWritingMatrix(m);
end;

m:=IdentityMat(10);;
RunMatTest("integer matrix", m);
m:=IdentityMat(10,GF(257));; # plain list of plain lists
RunMatTest("GF(257) plist-of-plist matrix", m);

m:=IdentityMat(10,GF(2));;
m:=IdentityMatrix(GF(257), 10); # IsPlistMatrixRep
RunMatTest("GF(257) IsPlistMatrixRep", m);

m:=IdentityMatrix(GF(257), 10); # IsGenericMatrixRep
RunMatTest("GF(257) IsGenericMatrixRep", m);

m:=IdentityMat(10,GF(2));; # plain list of IsGF2VectorRep
RunMatTest("GF(2) rowlist", m);

m:=IdentityMat(10,GF(2));; ConvertToMatrixRep(m);;
m:=IdentityMatrix(GF(2), 10); # IsGF2MatrixRep
RunMatTest("GF(2) compressed matrix", m);

m:=IdentityMat(10,GF(7));;
m:=IdentityMat(10,GF(7));; # plain list of Is8BitVectorRep
RunMatTest("GF(7) rowlist", m);

m:=IdentityMat(10,GF(7));; ConvertToMatrixRep(m);;
m:=IdentityMatrix(GF(7), 10); # Is8BitMatrixRep
RunMatTest("GF(7) compressed matrix", m);
13 changes: 9 additions & 4 deletions benchmark/matobj/bench-matobj-creation.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,18 +91,23 @@ RunMatTest := function(desc, ring)
TestCreatingMatrix(ring);
end;

RunMatTest("GF(2)", GF(2));
RunMatTest("Rationals", Rationals);

RunMatObjTest := function(desc, filter, ring)
Print("\n");
PrintBoxed(Concatenation("Testing ", desc));
TestCreatingMatrixObj(filter, ring);
end;

RunMatObjTest("GF(2) IsPlistMatrixRep", IsPlistMatrixRep, GF(2));
RunMatTest("GF(257)", GF(257));
RunMatObjTest("GF(257) IsPlistMatrixRep", IsPlistMatrixRep, GF(257));
RunMatObjTest("GF(257) IsGenericMatrixRep", IsGenericMatrixRep, GF(257));

RunMatTest("Integers", Integers);
RunMatObjTest("integer IsPlistMatrixRep", IsPlistMatrixRep, Integers);
RunMatObjTest("integer IsGenericMatrixRep", IsGenericMatrixRep, Integers);

RunMatTest("Rationals", Rationals);
RunMatObjTest("rational IsPlistMatrixRep", IsPlistMatrixRep, Rationals);
RunMatObjTest("rational IsGenericMatrixRep", IsGenericMatrixRep, Rationals);

# TODO: other reps
# TODO: other compare with creating plist-of-plist
1 change: 1 addition & 0 deletions doc/ref/makedocreldata.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ GAPInfo.ManualDataRef:= rec(
"../../lib/matobj2.gd",
"../../lib/matobjnz.gd",
"../../lib/matobjplist.gd",
"../../lib/matobjgeneric.gd",
"../../lib/matrix.gd",
"../../lib/meataxe.gi",
"../../lib/memory.gd",
Expand Down
1 change: 1 addition & 0 deletions doc/ref/matobj.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ described in this chapter is supported.
<#Include Label="IsGF2MatrixRep">
<#Include Label="Is8BitMatrixRep">
<#Include Label="IsPlistMatrixRep">
<#Include Label="IsGenericMatrixRep">
<#Include Label="IsZmodnZMatrixRep">

</Section>
Expand Down
46 changes: 46 additions & 0 deletions lib/matobjgeneric.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#############################################################################
##
## This file is part of GAP, a system for computational discrete algebra.
##
## SPDX-License-Identifier: GPL-2.0-or-later
##
## Copyright of GAP belongs to its developers, whose names are too numerous
## to list here. Please refer to the COPYRIGHT file for details.
##

############################################################################
#
# Dense matrix objects backed by plain lists of plain row lists.
#

#############################################################################
##
## <#GAPDoc Label="IsGenericMatrixRep">
## <ManSection>
## <Filt Name="IsGenericMatrixRep" Arg='obj' Type="Representation"/>
##
## <Description>
## An object <A>obj</A> in <Ref Filt="IsGenericMatrixRep"/> describes
## a matrix object (see <Ref Filt="IsMatrixObj"/>) whose entries are stored
## as a dense plain list of dense plain row lists.
Copy link
Copy Markdown
Contributor

@james-d-mitchell james-d-mitchell Apr 20, 2026

Choose a reason for hiding this comment

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

It'd be good to clarify what the difference is between IsPlistMatrixRep and IsGenericMatrixRep is here, as described in one of the comments elsewhere.

Copy link
Copy Markdown
Contributor

@james-d-mitchell james-d-mitchell Apr 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

For example, although I know what is intended "a dense plain list of dense plain row lists" and "is not a row list matrix" seem contradictory. More words might indicate more clearly what is meant here.

## <P/>
## This representation is optimized for efficient entry access via
## <M>M[i,j]</M>. Unlike <Ref Filt="IsPlistMatrixRep"/>, it is not a row
## list matrix, so direct row access via <M>M[i]</M> is not supported.
## </Description>
## </ManSection>
## <#/GAPDoc>
##
DeclareRepresentation( "IsGenericMatrixRep",
IsMatrixObj and IsPositionalObjectRep
and IsCopyable
and IsNoImmediateMethodsObject
and HasNumberRows and HasNumberColumns
and HasBaseDomain and HasOneOfBaseDomain and HasZeroOfBaseDomain,
[] );


# Internal positions for flat plist matrices.
Comment thread
fingolfin marked this conversation as resolved.
Outdated
BindConstant( "FBDPOS", 1 );
Copy link
Copy Markdown
Contributor

@james-d-mitchell james-d-mitchell Apr 20, 2026

Choose a reason for hiding this comment

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

Does the leading "F" stand for "Flat"? If so it should probably be "G" for generic no?

BindConstant( "FCOLSPOS", 2 );
Copy link
Copy Markdown
Contributor

@james-d-mitchell james-d-mitchell Apr 20, 2026

Choose a reason for hiding this comment

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

Is there a reason that this exists? It seems to only be used in NumberColumns couldn't is just be Length(m![FROWSPOS][1]) instead? Or I suppose

if Length(m![FROWSPOS]) = 0 then
return 0;
else
return Length(m![FROWSPOS][1]);
fi;

If this is for performance reasons, then maybe a comment indicating that?

Copy link
Copy Markdown
Member Author

@fingolfin fingolfin Apr 20, 2026

Choose a reason for hiding this comment

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

Then we'd loose support for 0 x m matrices, which is really annoying in lots of situations in my practical experience.

Copy link
Copy Markdown
Contributor

@james-d-mitchell james-d-mitchell Apr 21, 2026

Choose a reason for hiding this comment

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

Ah, I see, and I agree that 0xm matrices are useful. Maybe add a comment about this?

BindConstant( "FROWSPOS", 3 );
Loading
Loading