Skip to content

Widen declaration for many matrix related functions#6095

Open
fingolfin wants to merge 1 commit into
masterfrom
mh/widen-matrix-to-matrixobj
Open

Widen declaration for many matrix related functions#6095
fingolfin wants to merge 1 commit into
masterfrom
mh/widen-matrix-to-matrixobj

Conversation

@fingolfin
Copy link
Copy Markdown
Member

@fingolfin fingolfin commented Sep 5, 2025

The main motivation of course is to be able to use more MatrixObj stuff.

Also remove some now redundant methods.

Before:

gap> m:=RandomMat(256, 256, GF(1000000007));;
gap> mat:=Matrix(IsZmodnZMatrixRep, GF(1000000007), m);;
gap> RankMat(mat)=256; time;
true
170
gap> DeterminantMat(mat); time;
ZmodpZObj( 873487643, 1000000007 )
23982

After:

gap> m:=RandomMat(256, 256, GF(1000000007));;
gap> mat:=Matrix(IsZmodnZMatrixRep, GF(1000000007), m);;
gap> RankMat(mat)=256; time;
true
161
gap> DeterminantMat(mat); time;
ZmodpZObj( 873487643, 1000000007 )
1949

As discussed on #6088

Copy link
Copy Markdown
Member

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

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

Seems fine to me.

The diffs in the manual examples are harmless, as far as I can tell (the relevant groups are equal, they're just given by different generating sets).

@wilfwilson
Copy link
Copy Markdown
Member

There's still one pesky harmless failing manual example 🙁

@fingolfin fingolfin force-pushed the mh/widen-matrix-to-matrixobj branch 2 times, most recently from d8b8c29 to ae66eef Compare January 19, 2026 10:43
Also remove some now redundant methods.

Before:

    gap> m:=RandomMat(256, 256, GF(1000000007));;
    gap> mat:=Matrix(IsZmodnZMatrixRep, GF(1000000007), m);;
    gap> RankMat(mat)=256; time;
    true
    170
    gap> DeterminantMat(mat); time;
    ZmodpZObj( 873487643, 1000000007 )
    23982

After:

    gap> m:=RandomMat(256, 256, GF(1000000007));;
    gap> mat:=Matrix(IsZmodnZMatrixRep, GF(1000000007), m);;
    gap> RankMat(mat)=256; time;
    true
    161
    gap> DeterminantMat(mat); time;
    ZmodpZObj( 873487643, 1000000007 )
    1949
@fingolfin fingolfin force-pushed the mh/widen-matrix-to-matrixobj branch from ae66eef to c6d1276 Compare June 5, 2026 16:26
@fingolfin fingolfin requested a review from ThomasBreuer June 5, 2026 16:26
@fingolfin fingolfin added topic: performance bugs or enhancements related to performance (improvements or regressions) topic: library release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Jun 5, 2026
@fingolfin fingolfin added this to the GAP 4.17.0 milestone Jun 5, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.88%. Comparing base (74a43d6) to head (c6d1276).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6095      +/-   ##
==========================================
+ Coverage   78.86%   78.88%   +0.01%     
==========================================
  Files         685      685              
  Lines      293550   293542       -8     
  Branches     8672     8672              
==========================================
+ Hits       231515   231556      +41     
+ Misses      60225    60177      -48     
+ Partials     1810     1809       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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 topic: performance bugs or enhancements related to performance (improvements or regressions)

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants