From 3886bb78d4b6e489fcf260fad17b7824a95002b0 Mon Sep 17 00:00:00 2001 From: Guillaume Marques Date: Wed, 2 Aug 2023 12:19:57 +0200 Subject: [PATCH] check JET errors in view --- src/matrix.jl | 28 +++++++++++++++++++++++----- src/pcsr.jl | 35 ++++++++++++++++++++++++----------- src/views.jl | 4 +++- test/unit/unitests.jl | 2 ++ test/unit/views.jl | 43 +++++++++++++++++++++++++++++++++++++------ 5 files changed, 89 insertions(+), 23 deletions(-) diff --git a/src/matrix.jl b/src/matrix.jl index bb1ce8c..649e0cf 100644 --- a/src/matrix.jl +++ b/src/matrix.jl @@ -46,10 +46,17 @@ function Base.setindex!(m::DynamicSparseMatrix{K,L,T}, val, row::K, col::L) wher m.n = max(m.n, col) end if m.fillmode - addelem!(m.buffer, row, col, val) + buffer = m.buffer + @assert !isnothing(buffer) + addelem!(buffer, row, col, val) else - m.colmajor[row, col] = val - m.rowmajor[col, row] = val + colmajor = m.colmajor + @assert !isnothing(colmajor) + colmajor[row, col] = val + + rowmajor = m.rowmajor + @assert !isnothing(rowmajor) + rowmajor[col, row] = val end return m end @@ -61,12 +68,23 @@ function Base.getindex(m::DynamicSparseMatrix, row, col) end function Base.view(m::DynamicSparseMatrix{K,L,T}, row::K, ::Colon) where {K,L,T} - return m.fillmode ? view(m.buffer, row, :) : view(m.rowmajor, :, row) + if m.fillmode + # Do not allow to create a view on a buffer, otherwise the method is type unstable. + error("Matrix is in fill mode, cannot create a view. However, you can use the view method on the buffer.") + # buffer = m.buffer + # @assert !isnothing(buffer) + # return view(buffer, row, :) + end + rowmajor = m.rowmajor + @assert !isnothing(rowmajor) + return view(rowmajor, :, row) end function Base.view(m::DynamicSparseMatrix{K,L,T}, ::Colon, col::L) where {K,L,T} m.fillmode && error("View of a column not available in fill mode.") - return view(m.colmajor, :, col) + colmajor = m.colmajor + @assert !isnothing(colmajor) + return view(colmajor, :, col) end Base.ndims(m::DynamicSparseMatrix) = 2 diff --git a/src/pcsr.jl b/src/pcsr.jl index dcddcd6..b64759b 100644 --- a/src/pcsr.jl +++ b/src/pcsr.jl @@ -112,25 +112,30 @@ function addpartition!(pcsc::PackedCSC{K,T}) where {K,T} end function addpartition!(pcsc::PackedCSC{K,T}, prev_sem_id::Int) where {K,T} + semaphores = pcsc.semaphores + @assert !isnothing(semaphores) sem_key = semaphore_key(K) - nb_semaphores = length(pcsc.semaphores) + nb_semaphores = length(semaphores) sem_pos = 0 - if pcsc.semaphores[prev_sem_id + 1] === nothing - next_sem_id = _nextnonemptypos(pcsc.semaphores, prev_sem_id + 1) - sem_pos = pcsc.semaphores[next_sem_id] - 1 # insert the new semaphore in the pma.array just before the next one + semaphore_target = semaphores[prev_sem_id + 1] + if semaphore_target === nothing + next_sem_id = _nextnonemptypos(semaphores, prev_sem_id + 1) + next_semaphore = semaphores[next_sem_id] + @assert !isnothing(next_semaphore) + sem_pos = next_semaphore - 1 # insert the new semaphore in the pma.array just before the next one else - sem_pos = pcsc.semaphores[prev_sem_id + 1] - 1 # insert the new semaphore just before the next one - resize!(pcsc.semaphores, nb_semaphores + 1) # create room for the position of the new semaphore + sem_pos = semaphore_target - 1 # insert the new semaphore just before the next one + resize!(semaphores, nb_semaphores + 1) # create room for the position of the new semaphore for i in nb_semaphores:-1:(prev_sem_id+1) moved_sem_pos = pcsc.semaphores[i] - pcsc.semaphores[i+1] = pcsc.semaphores[i] + semaphores[i+1] = semaphores[i] pcsc.pma.array[moved_sem_pos] = (sem_key, T(i+1)) end end pcsc.nb_partitions += 1 sem_val = T(prev_sem_id+1) insert_pos, new_elem = _insert!(pcsc.pma.array, sem_key, sem_val, sem_pos, pcsc.semaphores) - pcsc.semaphores[prev_sem_id+1] = insert_pos + semaphores[prev_sem_id+1] = insert_pos if new_elem pcsc.pma.nb_elements += 1 win_start, win_end, nbcells = _look_for_rebalance!(pcsc.pma, insert_pos) @@ -162,12 +167,19 @@ function addcolumn!(mpcsc::MappedPackedCSC{K,L,T}, col::L, prev_col_pos::Int) wh return col_pos end -_pos_of_partition_start(pcsc, partition) = pcsc.semaphores[partition] +function _pos_of_partition_start(pcsc, partition) + partition_start_pos = pcsc.semaphores[partition] + @assert !isnothing(partition_start_pos) + return partition_start_pos +end + function _pos_of_partition_end(pcsc, partition) pos = length(pcsc.pma.array) next_partition = _nextnonemptypos(pcsc.semaphores, partition) if next_partition != 0 - pos = pcsc.semaphores[next_partition] - 1 + next_partition_start_pos = pcsc.semaphores[next_partition] + @assert !isnothing(next_partition_start_pos) + pos = next_partition_start_pos - 1 end return pos end @@ -343,7 +355,8 @@ function _dynamicsparse( ) where {K,L,T} !always_use_map && error("TODO issue #2.") - p = sortperm(collect(zip(J,I)), alg=QuickSort) # Columns first + ind = collect(zip(J,I)) + p = sortperm(ind, alg=QuickSort) # Columns first @inbounds I = I[p] @inbounds J = J[p] @inbounds V = V[p] diff --git a/src/views.jl b/src/views.jl index 49f117a..40c839a 100644 --- a/src/views.jl +++ b/src/views.jl @@ -52,7 +52,9 @@ function Base.view(::Buffer{L,K,T}, ::Colon, col::L) where {K,L,T} throw(ArgumentError("Cannot view a column of the BufferView.")) end -function Base.iterate(bf::BufferView, state = 1) +Base.eltype(::Type{BufferView{L,K,T}}) where {K,L,T} = Tuple{L,T} +Base.length(bf::BufferView) = length(bf.colids) +function Base.iterate(bf::BufferView, state::Int = 1) state > length(bf.vals) && return nothing return ((bf.colids[state], bf.vals[state]), state + 1) end \ No newline at end of file diff --git a/test/unit/unitests.jl b/test/unit/unitests.jl index 349d30f..037450f 100644 --- a/test/unit/unitests.jl +++ b/test/unit/unitests.jl @@ -46,6 +46,8 @@ function unit_tests() @testset "Views" begin test_views() + test_buffer_views() + @test_call test_buffer_views() end @testset "Sparse Matrix Vector Multiplication" begin diff --git a/test/unit/views.jl b/test/unit/views.jl index 8f387fe..f3a3c3b 100644 --- a/test/unit/views.jl +++ b/test/unit/views.jl @@ -1,9 +1,4 @@ -function test_views() - I = [1, 1, 2, 4, 3, 5, 1, 4, 1, 5, 1, 5, 4, 4, 3, 9, 1] - J = [4, 3, 3, 7, 18, 9, 3, 18, 4, 2, 3, 1, 7, 3, 3, 3, 18] - V = [1, 8, 10, 2, -5, 3, 2, 1, 1, 1, 5, 3, 2, 1, 7, 8, 1] - matrix = dynamicsparse(I,J,V) - +function _test_views(matrix) # get the row with id 5 ids = Int[] vals = Int[] @@ -33,4 +28,40 @@ function test_views() end @test ids == [1,3,4] @test vals == [1,-5,1] + return +end + +function test_views() + I = [1, 1, 2, 4, 3, 5, 1, 4, 1, 5, 1, 5, 4, 4, 3, 9, 1] + J = [4, 3, 3, 7, 18, 9, 3, 18, 4, 2, 3, 1, 7, 3, 3, 3, 18] + V = [1, 8, 10, 2, -5, 3, 2, 1, 1, 1, 5, 3, 2, 1, 7, 8, 1] + matrix = dynamicsparse(I,J,V) + + _test_views(matrix) + @test_call _test_views(matrix) +end + +function test_buffer_views() + matrix = dynamicsparse(Int, Int, Int) + + matrix[1,2] = 1 + matrix[2,1] = 2 + matrix[2,2] = 3 + matrix[3,1] = 4 + matrix[3,2] = 5 + matrix[1,7] = 3 + + buffer = matrix.buffer + @assert !isnothing(buffer) + + ids = Int[] + vals = Int[] + for (id, val) in view(buffer, 1, :) + push!(ids, id) + push!(vals, val) + end + + @test ids == [2, 7] + @test vals == [1, 3] + return end