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

Add GAP_IsMatrix #4531

Merged
merged 2 commits into from
Jun 5, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
20 changes: 14 additions & 6 deletions src/libgap-api.c
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ Obj GAP_NewPlist(Int capacity)
////

static Obj IsMatrixOrMatrixObjFilt;
static Obj IsMatrixFilt;
static Obj IsMatrixObjFilt;
static Obj NrRowsAttr;
static Obj NrColsAttr;
Expand All @@ -422,6 +423,12 @@ int GAP_IsMatrixOrMatrixObj(Obj obj)
return obj && DoFilter(IsMatrixOrMatrixObjFilt, obj) == True;
}

// Returns 1 if <obj> is a GAP matrix, 0 if not.
int GAP_IsMatrix(Obj obj)
{
return obj && DoFilter(IsMatrixFilt, obj) == True;
Copy link
Member

Choose a reason for hiding this comment

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

IsMatrix is not a plain filter, but rather an and-filter, so calling DoFilter is wrong and the cause for the crash in the CI tests.

Suggested change
return obj && DoFilter(IsMatrixFilt, obj) == True;
return obj && CALL_1ARGS(IsMatrixFilt, obj) == True;

I suggest we change GAP_IsMatrixOrMatrixObj and GAP_IsMatrixObj similarly -- while DoFilter works for them right now, it's a bit risky, as any change to their definition would accidentally break this code.

}

// Returns 1 if <obj> is a GAP matrix obj, 0 if not.
int GAP_IsMatrixObj(Obj obj)
{
Expand All @@ -436,17 +443,17 @@ UInt GAP_NrRows(Obj mat)
return UInt_ObjInt(nrows);
}

// Returns the number of columns of the given GAP matrix obj.
// If <mat> is not a GAP matrix obj, an error may be raised.
// Returns the number of columns of the given GAP matrix or matrix obj.
// If <mat> is not a GAP matrix or matrix obj, an error may be raised.
UInt GAP_NrCols(Obj mat)
{
Obj ncols = CALL_1ARGS(NrColsAttr, mat);
return UInt_ObjInt(ncols);
}

// Assign <val> at position <pos> into the GAP matrix obj <mat>.
// Assign <val> at the <row>, <col> into the GAP matrix or matrix obj <mat>.
// If <val> is zero, then this unbinds the list entry.
// If <mat> is not a GAP matrix obj, an error may be raised.
// If <mat> is not a GAP matrix or matrix obj, an error may be raised.
void GAP_AssMat(Obj mat, UInt row, UInt col, Obj val)
{
Obj r = ObjInt_UInt(row);
Expand All @@ -456,8 +463,8 @@ void GAP_AssMat(Obj mat, UInt row, UInt col, Obj val)

// Returns the element at the <row>, <col> in the GAP matrix obj <mat>.
// Returns 0 if <row> or <col> are out of bounds, i.e., if either
// is zero, or larger than the number of rows respectively columns of the list.
// If <mat> is not a GAP matrix obj, an error may be raised.
// is zero, or larger than the number of rows respectively columns of <mat>.
// If <mat> is not a GAP matrix or matrix obj, an error may be raised.
Obj GAP_ElmMat(Obj mat, UInt row, UInt col)
{
Obj r = ObjInt_UInt(row);
Expand Down Expand Up @@ -616,6 +623,7 @@ void GAP_Error_Postjmp_Returning_(void)
static Int InitKernel(StructInitInfo * module)
{
InitFopyGVar("IsMatrixOrMatrixObj", &IsMatrixOrMatrixObjFilt);
InitFopyGVar("IsMatrix", &IsMatrixFilt);
InitFopyGVar("IsMatrixObj", &IsMatrixObjFilt);
InitFopyGVar("NrRows", &NrRowsAttr);
InitFopyGVar("NrCols", &NrColsAttr);
Expand Down
21 changes: 17 additions & 4 deletions src/libgap-api.h
Original file line number Diff line number Diff line change
Expand Up @@ -374,18 +374,31 @@ Obj GAP_NewPlist(Int capacity);
//// matrix obj
////

// Note that the meaning of the following filters is not self-explanatory,
// see the chapter "Vector and Matrix Objects" in the GAP Reference Manual.
// `GAP_IsMatrixOrMatrixObj` checks whether the argument is an abstract
// 2-dim. array; such objects admit access to entries via `GAP_ElmMat`,
// one can ask for the numbers of rows and columns via `GAP_NrRows` and
// `GAP_NrCols`, respectively, etc.
// `GAP_IsMatrix` checks for special cases that are nonempty list of lists
// with additional properties; often these are plain lists.
// `GAP_IsMatrixObj` checks for special cases that are not plain lists.

// Returns 1 if <obj> is a GAP matrix or matrix obj, 0 if not.
int GAP_IsMatrixOrMatrixObj(Obj obj);

// Returns 1 if <obj> is a GAP matrix, 0 if not.
int GAP_IsMatrix(Obj obj);

// Returns 1 if <obj> is a GAP matrix obj, 0 if not.
int GAP_IsMatrixObj(Obj obj);

// Returns the number of rows of the given GAP matrix obj.
// If <mat> is not a GAP matrix obj, an error may be raised.
// Returns the number of rows of the given GAP matrix or matrix obj.
// If <mat> is not a GAP matrix or matrix obj, an error may be raised.
UInt GAP_NrRows(Obj mat);

// Returns the number of columns of the given GAP matrix obj.
// If <mat> is not a GAP matrix obj, an error may be raised.
// Returns the number of columns of the given GAP matrix or matrix obj.
// If <mat> is not a GAP matrix or matrix obj, an error may be raised.
UInt GAP_NrCols(Obj mat);

// Assign <val> at position <pos> into the GAP matrix obj <mat>.
Expand Down
5 changes: 5 additions & 0 deletions tst/testlibgap/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,24 @@ void matrices(void)

assert(!GAP_IsMatrixOrMatrixObj(mat)); // empty list, not yet a matrix
assert(!GAP_IsMatrixObj(mat));
assert(!GAP_IsMatrix(mat));
assert(!GAP_IsMatrixOrMatrixObj(0));
assert(!GAP_IsMatrixObj(0));
assert(!GAP_IsMatrix(0));
assert(!GAP_IsMatrixOrMatrixObj(val));
assert(!GAP_IsMatrixObj(val));
assert(!GAP_IsMatrix(val));

row = GAP_NewPlist(2);
GAP_AssList(row, 1, INTOBJ_INT(1));
GAP_AssList(row, 2, INTOBJ_INT(2));
GAP_AssList(mat, 1, row);
assert(!GAP_IsMatrixOrMatrixObj(row));
assert(!GAP_IsMatrixObj(row));
assert(!GAP_IsMatrix(row));
assert(GAP_IsMatrixOrMatrixObj(mat));
assert(!GAP_IsMatrixObj(mat)); // list of lists, not proper matrix object
assert(GAP_IsMatrix(mat));

GAP_AssMat(mat, 1, 1, val);
ret = GAP_ElmMat(mat, 1, 1);
Expand Down