From 6394e8e95b426b795177f589f9fcc9aac0b95766 Mon Sep 17 00:00:00 2001 From: "Viral B. Shah" Date: Mon, 7 Sep 2026 19:30:07 -0400 Subject: [PATCH] Fix `isone` for sparse matrices with stored zeros `isone(A::AbstractSparseMatrixCSC)` only validated the stored entries it encountered and relied on `nnz(A) >= n` as a proxy for "every column has a diagonal entry". A matrix with a stored zero and a missing diagonal entry therefore passed, e.g. `sparse([1 0; 1 1]) * sparse([1 0; -1 0])`. Track per column whether a stored diagonal entry equal to one was seen, and return `false` otherwise. Fixes #763 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XzN6CtuVBJWDVc88ShNYso --- src/sparsematrix.jl | 16 +++++++++++++--- test/sparsematrix_ops.jl | 6 ++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/sparsematrix.jl b/src/sparsematrix.jl index 56ee216a..93bb439d 100644 --- a/src/sparsematrix.jl +++ b/src/sparsematrix.jl @@ -2237,9 +2237,19 @@ Base.iszero(A::AbstractSparseMatrixCSC) = iszero(nzvalview(A)) function Base.isone(A::AbstractSparseMatrixCSC) m, n = size(A) m == n && getcolptr(A)[n+1] >= n+1 || return false - for j in axes(A,2), k in getcolptr(A)[j]:(getcolptr(A)[j+1] - 1) - i, x = rowvals(A)[k], nonzeros(A)[k] - ifelse(i == j, isone(x), iszero(x)) || return false + for j in axes(A,2) + founddiag = false + for k in getcolptr(A)[j]:(getcolptr(A)[j+1] - 1) + i, x = rowvals(A)[k], nonzeros(A)[k] + if i == j + isone(x) || return false + founddiag = true + else + iszero(x) || return false + end + end + # every column must have a stored diagonal entry equal to one + founddiag || return false end return true end diff --git a/test/sparsematrix_ops.jl b/test/sparsematrix_ops.jl index ab2065cb..ee254dce 100644 --- a/test/sparsematrix_ops.jl +++ b/test/sparsematrix_ops.jl @@ -46,6 +46,12 @@ end @test !isone(spzeros(3, 3)) # test failure for too few stored entries @test !isone(sparse(2I, 3, 3)) # test failure for non-one diagonal entries @test !isone(sparse(Bidiagonal(fill(1, 3), fill(1, 2), :U))) # test failure for non-zero off-diag entries + # issue #763: stored zeros must not be counted towards the diagonal + M = sparse([1 0; 1 1]) * sparse([1 0; -1 0]) + @test nnz(M) == 2 && !isone(M) && !isone(Matrix(M)) + @test !isone(SparseMatrixCSC(2, 2, [1, 3, 3], [1, 2], [1, 0])) + @test !isone(SparseMatrixCSC(2, 2, [1, 2, 3], [1, 1], [1, 0])) + @test isone(SparseMatrixCSC(2, 2, [1, 3, 4], [1, 2, 2], [1, 0, 1])) # stored zero off-diagonal is fine end @testset "indtype" begin