Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[BlockSparseArrays] Define in-place view that may instantiate blocks #1498

Merged
merged 8 commits into from
Jun 14, 2024

Conversation

mtfishman
Copy link
Member

@mtfishman mtfishman commented Jun 13, 2024

To-do:

  • Investigate issue with too many blocks being allocated when performing block sparse matrix multiplication.

As discussed in ITensor/BlockSparseArrays.jl#2. For example:

julia> using BlockArrays: Block

julia> using NDTensors.BlockSparseArrays: BlockSparseArray, @view!

julia> a = BlockSparseArray{Float64}([2, 3], [2, 3])
typeof(axes) = Tuple{BlockArrays.BlockedOneTo{Int64, Vector{Int64}}, BlockArrays.BlockedOneTo{Int64, Vector{Int64}}}

Warning: To temporarily circumvent a bug in printing BlockSparseArrays with mixtures of dual and non-dual axes, the types of the dual axes printed below might not be accurate. The types printed above this message are the correct ones.

2×2-blocked 5×5 BlockSparseArray{Float64, 2, Matrix{Float64}, NDTensors.SparseArrayDOKs.SparseArrayDOK{Matrix{Float64}, 2, NDTensors.BlockSparseArrays.BlockZero{Tuple{BlockArrays.BlockedOneTo{Int64, Vector{Int64}}, BlockArrays.BlockedOneTo{Int64, Vector{Int64}}}}}, Tuple{BlockArrays.BlockedOneTo{Int64, Vector{Int64}}, BlockArrays.BlockedOneTo{Int64, Vector{Int64}}}}:
 0.0  0.00.0  0.0  0.0
 0.0  0.00.0  0.0  0.0
 ──────────┼───────────────
 0.0  0.00.0  0.0  0.0
 0.0  0.00.0  0.0  0.0
 0.0  0.00.0  0.0  0.0

julia> b = @view! a[Block(2, 2)] # or: `b = view!(a, Block(2, 2))`
3×3 Matrix{Float64}:
 0.0  0.0  0.0
 0.0  0.0  0.0
 0.0  0.0  0.0

julia> b[2, 2] = 2
2

julia> a
typeof(axes) = Tuple{BlockArrays.BlockedOneTo{Int64, Vector{Int64}}, BlockArrays.BlockedOneTo{Int64, Vector{Int64}}}

Warning: To temporarily circumvent a bug in printing BlockSparseArrays with mixtures of dual and non-dual axes, the types of the dual axes printed below might not be accurate. The types printed above this message are the correct ones.

2×2-blocked 5×5 BlockSparseArray{Float64, 2, Matrix{Float64}, NDTensors.SparseArrayDOKs.SparseArrayDOK{Matrix{Float64}, 2, NDTensors.BlockSparseArrays.BlockZero{Tuple{BlockArrays.BlockedOneTo{Int64, Vector{Int64}}, BlockArrays.BlockedOneTo{Int64, Vector{Int64}}}}}, Tuple{BlockArrays.BlockedOneTo{Int64, Vector{Int64}}, BlockArrays.BlockedOneTo{Int64, Vector{Int64}}}}:
 0.0  0.00.0  0.0  0.0
 0.0  0.00.0  0.0  0.0
 ──────────┼───────────────
 0.0  0.00.0  0.0  0.0
 0.0  0.00.0  2.0  0.0
 0.0  0.00.0  0.0  0.0

julia> b
3×3 Matrix{Float64}:
 0.0  0.0  0.0
 0.0  2.0  0.0
 0.0  0.0  0.0

This can be better to work with than view(a, Block(2, 2))/@view a[Block(2, 2)] since as of #1481, view(::BlockSparseArray, ...) creates a SubArray wrapper around the entire BlockSparseArray instead of returning the block data directly, in order to uniformly handle stored and unstored blocks:

julia> @view a[Block(2, 2)]
3×3 view(::BlockSparseArray{Float64, 2, Matrix{Float64}, NDTensors.SparseArrayDOKs.SparseArrayDOK{Matrix{Float64}, 2, NDTensors.BlockSparseArrays.BlockZero{Tuple{BlockArrays.BlockedOneTo{Int64, Vector{Int64}}, BlockArrays.BlockedOneTo{Int64, Vector{Int64}}}}}, Tuple{BlockArrays.BlockedOneTo{Int64, Vector{Int64}}, BlockArrays.BlockedOneTo{Int64, Vector{Int64}}}}, BlockSlice(Block(2),3:5), BlockSlice(Block(2),3:5)) with eltype Float64:
 0.0  0.0  0.0
 0.0  2.0  0.0
 0.0  0.0  0.0

Also note that sub-slicing of blocks is also supported with the syntax @view! a[Block(2, 2)[1:2, 1:2]] or view!(a, Block(2, 2)[1:2, 1:2]), in which case it outputs a view of the block data (either the existing block data or the newly allocated block data):

julia> @view! a[Block(2, 2)[1:2, 1:2]]
2×2 view(::Matrix{Float64}, 1:2, 1:2) with eltype Float64:
 0.0  0.0
 0.0  2.0

@ogauthe @emstoudenmire

@mtfishman
Copy link
Member Author

mtfishman commented Jun 13, 2024

@ogauthe hopefully by using view!/@view! that can circumvent the issues you were seeing passing blocks/block slices to TensorAlgebra.contract and Strided.@strided (which for future reference are listed in ITensor/BlockSparseArrays.jl#2). Of course it would be ideal in the longer run to fix whatever is practical to fix when view/@view is used but this should provide a simple and reliable workaround in the short term.

This change introduced an issue that more more blocks than necessary are being allocated in the result of block sparse matrix multiplication, I'll have to investigate that before merging this. EDIT: It turned out that fill!(a::BlockSparseArray, 0) an a::BlockSparseArray .= 0 were both allocating blocks filled with zeros, and ArrayLayouts.jl, which we are using for a matrix multiplication interface in BlockSparseArrays and SparseArrayDOKs, calls fill!(a, 0) at some point in the matrix multiplication call stack. Now both operations drop all blocks from a which fixed that issue.

@emstoudenmire
Copy link
Collaborator

Looks really nice

@mtfishman mtfishman changed the title [BlockSparseArrays] Define in-place view to maybe instantiate blocks [BlockSparseArrays] Define in-place view that may instantiate blocks Jun 14, 2024
@mtfishman mtfishman merged commit 87ec605 into main Jun 14, 2024
15 of 16 checks passed
@mtfishman mtfishman deleted the BlockSparseArrays_inplace_view branch June 14, 2024 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants