Optimize is_zero_row/_column (#1802)

... by avoiding redundant bounds checks. Also change the methods as well as that one for is_zero_entry to apply to Julia matrices, too.
Nemocas · Sep 20, 2024 · 443e7a4 · 443e7a4
1 parent a37673c
commit 443e7a4
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 19 deletions.
diff --git a/src/Matrix.jl b/src/Matrix.jl
@@ -233,34 +233,36 @@ function isone(a::MatrixElem{T}) where T <: NCRingElement
 end
 
 @doc raw"""
-    is_zero_entry(M::MatrixElem{T}, i::Int, j::Int) where T <: NCRingElement
+    is_zero_entry(M::Union{Matrix,MatrixElem}, i::Int, j::Int)
 
 Return `true` if $M_{i,j}$ is zero.
 """
-@inline is_zero_entry(M::MatrixElem{T}, i::Int, j::Int) where T <: NCRingElement = iszero(M[i,j])
+@inline is_zero_entry(M::Union{Matrix,MatrixElem}, i::Int, j::Int) = iszero(M[i,j])
 
 @doc raw"""
-    is_zero_row(M::MatrixElem{T}, i::Int) where T <: NCRingElement
+    is_zero_row(M::Union{Matrix,MatrixElem}, i::Int)
 
 Return `true` if the $i$-th row of the matrix $M$ is zero.
 """
-function is_zero_row(M::MatrixElem{T}, i::Int) where T <: NCRingElement
+function is_zero_row(M::Union{Matrix,MatrixElem}, i::Int)
+  @boundscheck 1 <= i <= nrows(M) || Base.throw_boundserror(M, (i, 1:ncols(M)))
   for j in 1:ncols(M)
-    if !is_zero_entry(M, i, j)
+    @inbounds if !is_zero_entry(M, i, j)
       return false
     end
   end
   return true
 end
 
 @doc raw"""
-    is_zero_column(M::MatrixElem{T}, j::Int) where T <: NCRingElement
+    is_zero_column(M::Union{Matrix,MatrixElem}, j::Int)
 
 Return `true` if the $j$-th column of the matrix $M$ is zero.
 """
-function is_zero_column(M::MatrixElem{T}, j::Int) where T <: NCRingElement
+function is_zero_column(M::Union{Matrix,MatrixElem}, j::Int)
+  @boundscheck 1 <= j <= ncols(M) || Base.throw_boundserror(M, (1:nrows(M), j))
   for i in 1:nrows(M)
-    if !is_zero_entry(M, i, j)
+    @inbounds if !is_zero_entry(M, i, j)
       return false
     end
   end

diff --git a/src/julia/Matrix.jl b/src/julia/Matrix.jl
@@ -7,17 +7,6 @@
 number_of_rows(A::Matrix{T}) where {T} = size(A)[1]
 number_of_columns(A::Matrix{T}) where {T} = size(A)[2]
 
-function is_zero_row(M::Matrix, i::Int)
-    for j = 1:ncols(M)
-        if !iszero(M[i, j])
-            return false
-        end
-    end
-    return true
-end
-
-# TODO: add `is_zero_column(M::Matrix{T}, i::Int) where {T<:Integer}` and specialized functions in Nemo
-
 zero_matrix(::Type{Int}, r, c) = zeros(Int, r, c)
 
 ###############################################################################

diff --git a/test/generic/MatRing-test.jl b/test/generic/MatRing-test.jl
@@ -125,6 +125,11 @@ end
    @test is_zero_column(C, 2)
    @test !is_zero_column(C, 1)
 
+   @test_throws BoundsError is_zero_row(C, 0)
+   @test_throws BoundsError is_zero_row(C, 4)
+   @test_throws BoundsError is_zero_column(C, 0)
+   @test_throws BoundsError is_zero_column(C, 4)
+
    S = matrix_ring(QQ, 3)
    A = S([1 2 3; 4 5 6; 7 8 9])
 

diff --git a/test/generic/Matrix-test.jl b/test/generic/Matrix-test.jl
@@ -542,6 +542,11 @@ end
    @test is_zero_column(C, 2)
    @test !is_zero_column(C, 1)
 
+   @test_throws BoundsError is_zero_row(C, 0)
+   @test_throws BoundsError is_zero_row(C, 4)
+   @test_throws BoundsError is_zero_column(C, 0)
+   @test_throws BoundsError is_zero_column(C, 4)
+
    @test length(A) == length(B) == length(C) == 9
    @test !any(isempty, (A, B, C))