Skip to content

Validate DeterminantMat inputs#6328

Merged
fingolfin merged 1 commit intomasterfrom
codex/determinantmat-error-checking
Apr 17, 2026
Merged

Validate DeterminantMat inputs#6328
fingolfin merged 1 commit intomasterfrom
codex/determinantmat-error-checking

Conversation

@fingolfin
Copy link
Copy Markdown
Member

Fixes #5264

Co-authored-by: Codex codex@openai.com

Copy link
Copy Markdown
Contributor

@james-d-mitchell james-d-mitchell left a comment

Choose a reason for hiding this comment

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

Looks good, one minor comment

Comment thread lib/matrix.gi Outdated
m := NrRows(mat);
if m = 0 or m <> NrCols(mat) then
Error( "<mat> must be a square matrix at least 1x1" );
if m = 0 or not IsRectangularTable(mat) or m <> NrCols(mat) then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe better to leave the 1x1 in the error message?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Agreed.

Ideally we should simply support the determinant of 0x0 matrices (by the usual definitions, it must be equal to 1). But of course we can only do that for "true" matrixobj, where we know the underlying ring; for lists-of-lists, an "empty" matrix is just an empty list and we don't know over which ring it is.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

Reject ragged inputs before determinant dispatch.

Fixes #5264

Co-authored-by: Codex <codex@openai.com>
@fingolfin fingolfin force-pushed the codex/determinantmat-error-checking branch from 2bb5e67 to 59e45c1 Compare April 17, 2026 14:27
@fingolfin fingolfin added topic: library release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Apr 17, 2026
@fingolfin fingolfin enabled auto-merge (squash) April 17, 2026 14:28
@fingolfin fingolfin merged commit b6bbd05 into master Apr 17, 2026
32 checks passed
@fingolfin fingolfin deleted the codex/determinantmat-error-checking branch April 17, 2026 16:01
ThomasBreuer added a commit to ThomasBreuer/gap that referenced this pull request Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DeterminantMat produces 4 different types of behaviour on non-square-matrix inputs

2 participants