Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Avoid redundant bounds checks in is_zero_row and is_zero_column #1802

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

fingolfin
Copy link
Member

@fingolfin fingolfin commented Sep 19, 2024

Also change the methods as well as that one for is_zero_entry to apply to Julia matrices, too.

Copy link

codecov bot commented Sep 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.02%. Comparing base (0f40154) to head (78d9f59).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1802      +/-   ##
==========================================
+ Coverage   87.99%   88.02%   +0.02%     
==========================================
  Files         119      119              
  Lines       30073    30069       -4     
==========================================
+ Hits        26464    26467       +3     
+ Misses       3609     3602       -7     

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

... by avoiding redundant bounds checks.

Also change the methods as well as that one for is_zero_entry to
apply to Julia matrices, too.
@fingolfin
Copy link
Member Author

Measurements with this using ZZMatrix.

Before:

julia> @btime is_zero_row(A, 1);
  56.190 ns (0 allocations: 0 bytes)

julia> @btime is_zero_row(A, 2);
  32.780 ns (0 allocations: 0 bytes)

julia> @btime is_zero_row(A, 3);
  13.138 ns (0 allocations: 0 bytes)

After:

julia> A = zero_matrix(ZZ, 3, 100); A[2,50]=1; A[3,1] = 1;

julia> @btime is_zero_row(A, 1);
  43.012 ns (0 allocations: 0 bytes)

julia> @btime is_zero_row(A, 2);
  27.470 ns (0 allocations: 0 bytes)

julia> @btime is_zero_row(A, 3);
  11.678 ns (0 allocations: 0 bytes)

Copy link
Collaborator

@lgoettgens lgoettgens left a comment

Choose a reason for hiding this comment

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

LGTM, but let's wait for downstream tests to confirm

@lgoettgens lgoettgens closed this Sep 20, 2024
@lgoettgens lgoettgens reopened this Sep 20, 2024
@fingolfin fingolfin merged commit 443e7a4 into Nemocas:master Sep 20, 2024
58 of 60 checks passed
@fingolfin fingolfin deleted the mh/inbounds-is_zero_row branch September 20, 2024 20:24
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants