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

Improve InverseMatMod with integer modulus #2426

Merged

Conversation

fingolfin
Copy link
Member

Before:

gap> m2:=RandomUnimodularMat(100);;
gap> for i in [1..100] do x:=InverseMatMod(m2,2); od; time;
277
gap> for i in [1..100] do x:=InverseMatMod(m2,251); od; time;
1564
gap> for i in [1..100] do x:=InverseMatMod(m2,65537); od; time;
Error, reached the pre-set memory limit

After:

gap> m2:=RandomUnimodularMat(100);;
gap> for i in [1..100] do x:=InverseMatMod(m2,2); od; time;
273
gap> for i in [1..100] do x:=InverseMatMod(m2,251); od; time;
1249
gap> for i in [1..100] do x:=InverseMatMod(m2,65537); od; time;
1226

Before:

    gap> m2:=RandomUnimodularMat(100);;
    gap> for i in [1..100] do x:=InverseMatMod(m2,2); od; time;
    277
    gap> for i in [1..100] do x:=InverseMatMod(m2,251); od; time;
    1564
    gap> for i in [1..100] do x:=InverseMatMod(m2,65537); od; time;
    Error, reached the pre-set memory limit

After:

    gap> m2:=RandomUnimodularMat(100);;
    gap> for i in [1..100] do x:=InverseMatMod(m2,2); od; time;
    273
    gap> for i in [1..100] do x:=InverseMatMod(m2,251); od; time;
    1249
    gap> for i in [1..100] do x:=InverseMatMod(m2,65537); od; time;
    1226
@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: performance bugs or enhancements related to performance (improvements or regressions) labels Apr 30, 2018
@codecov
Copy link

codecov bot commented Apr 30, 2018

Codecov Report

Merging #2426 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2426      +/-   ##
==========================================
- Coverage   73.88%   73.88%   -0.01%     
==========================================
  Files         484      484              
  Lines      245462   245444      -18     
==========================================
- Hits       181368   181350      -18     
  Misses      64094    64094
Impacted Files Coverage Δ
lib/zlattice.gi 77.27% <100%> (-0.32%) ⬇️
src/objset.c 84.59% <0%> (-0.23%) ⬇️
hpcgap/lib/hpc/stdtasks.g 64.45% <0%> (+0.25%) ⬆️

@fingolfin fingolfin requested a review from ssiccha May 3, 2018 16:26
Copy link
Contributor

@hulpke hulpke left a comment

Choose a reason for hiding this comment

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

Yes, avoiding to pre-calculate the multiplication table should perform better, in particular if the prime is larger.

@fingolfin
Copy link
Member Author

It also avoids exhaustin the complete RAM ;-)

@ChrisJefferson ChrisJefferson merged commit b3fafa2 into gap-system:master May 4, 2018
@fingolfin fingolfin deleted the mh/improve-InverseMatMod branch May 4, 2018 21:03
@fingolfin fingolfin added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes topic: performance bugs or enhancements related to performance (improvements or regressions)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants