Skip to content

Commit 85f7b74

Browse files
adienescodex
andauthored
sparsematrix: fix swaprows! storage moves (#721)
reported on discourse, mostly oneshot by codex 5.5 ---------------------------------------------------------------------------------------------------------------- Fix SparseMatrixCSC row swaps when exactly one swapped row is stored in a column. The j search range used a local i index as an absolute storage index, and the single-row move rotations were reversed. On master: ```julia julia> A = sparse([1.0 2.0 3.0; 0.0 0.0 0.0; 4.0 5.0 6.0]); Base.swaprows!(A, 1, 2); A 3×3 SparseMatrixCSC{Float64, Int64} with 6 stored entries: ⋅ 1.0 ⋅ 2.0 ⋅ 3.0 4.0 5.0 6.0 julia> B = sparse(reshape([1.0, 2.0, 3.0, 4.0, 0.0, 0.0], 6, 1)); Base.swaprows!(B, 2, 6); rowvals(B) 4-element Vector{Int64}: 1 4 6 3 ``` --------- Co-authored-by: OpenAI Codex <codex@openai.com>
1 parent f7b1e19 commit 85f7b74

2 files changed

Lines changed: 20 additions & 7 deletions

File tree

src/sparsematrix.jl

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4513,7 +4513,7 @@ function Base.swaprows!(A::AbstractSparseMatrixCSC, i, j)
45134513
iidx = searchsortedfirst(@view(rows[rr]), i)
45144514
has_i = iidx <= length(rr) && rows[rr[iidx]] == i
45154515

4516-
jrange = has_i ? (iidx:last(rr)) : rr
4516+
jrange = has_i ? (rr[iidx]:last(rr)) : rr
45174517
jidx = searchsortedlast(@view(rows[jrange]), j)
45184518
has_j = jidx != 0 && rows[jrange[jidx]] == j
45194519

@@ -4527,18 +4527,16 @@ function Base.swaprows!(A::AbstractSparseMatrixCSC, i, j)
45274527
# Update the rowval and then rotate both nonzeros
45284528
# and the remaining rowvals into the correct place
45294529
rows[rr[iidx]] = j
4530-
jidx == 0 && continue
45314530
rotate_range = rr[iidx]:jrange[jidx]
4532-
circshift!(@view(vals[rotate_range]), 1)
4533-
circshift!(@view(rows[rotate_range]), 1)
4531+
circshift!(@view(vals[rotate_range]), -1)
4532+
circshift!(@view(rows[rotate_range]), -1)
45344533
else
45354534
# Same as i, but in the opposite direction
45364535
@assert has_j
45374536
rows[jrange[jidx]] = i
4538-
iidx > length(rr) && continue
45394537
rotate_range = rr[iidx]:jrange[jidx]
4540-
circshift!(@view(vals[rotate_range]), -1)
4541-
circshift!(@view(rows[rotate_range]), -1)
4538+
circshift!(@view(vals[rotate_range]), 1)
4539+
circshift!(@view(rows[rotate_range]), 1)
45424540
end
45434541
end
45444542
return nothing

test/sparsematrix_constructors_indexing.jl

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1426,6 +1426,21 @@ using Base: swaprows!, swapcols!
14261426
f!(Scopy, i, j); f!(Sdense, i, j)
14271427
@test Scopy == Sdense
14281428
end
1429+
1430+
for (A, i, j) in (
1431+
(sparse([1.0 2.0 3.0;
1432+
0.0 0.0 0.0;
1433+
4.0 5.0 6.0]), 1, 2),
1434+
(sparse([1.0 0.0 5.0;
1435+
0.0 2.0 0.0;
1436+
0.0 3.0 6.0;
1437+
7.0 4.0 0.0]), 1, 2),
1438+
(sparse(reshape([1.0, 2.0, 3.0, 4.0, 0.0, 0.0], 6, 1)), 2, 6))
1439+
Scopy = copy(A)
1440+
Sdense = Array(A)
1441+
swaprows!(Scopy, i, j); swaprows!(Sdense, i, j)
1442+
@test Scopy == Sdense
1443+
end
14291444
end
14301445

14311446
@testset "sprandn with type $T" for T in (Float64, Float32, Float16, ComplexF64, ComplexF32, ComplexF16)

0 commit comments

Comments
 (0)