Skip to content

Avoid ind2sub in view with Block{1} #268

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

Merged
merged 1 commit into from
Jun 22, 2023
Merged

Conversation

jishnub
Copy link
Member

@jishnub jishnub commented Jun 21, 2023

This avoids the internal function _ind2sub in constructing views of AbstractBlockArrays with one Block{1} index, and instead, indexes into a BlockRange to get the block index. As a consequence, a BlockBoundsError is changed to a BoundsError, but the error message is actually improved.
On master

(BlockArrays) julia> B = BlockArray(zeros(6,6), 1:3, 1:3);

(BlockArrays) julia> B[Block(10)]
ERROR: BlockBoundsError: attempt to access 3×3-blocked 6×6 BlockMatrix{Float64, Matrix{Matrix{Float64}}, Tuple{BlockedUnitRange{RangeCumsum{Int64, UnitRange{Int64}}}, BlockedUnitRange{RangeCumsum{Int64, UnitRange{Int64}}}}} at block index [1,4]

This PR

(BlockArrays) julia> B[Block(10)]
ERROR: BoundsError: attempt to access 3×3 BlockRange{2, Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}} at index [10]

The main part of the message is that the index [1,4] is changed to [10]. The [1,4] arises from a lack of bounds-checking in _ind2sub, and might be tricky to trace back.

Secondly, this PR introduces a constructor for BlockRange that accepts integer block sizes. This parallels CartesianIndices:

(BlockArrays) julia> collect(CartesianIndices((2,3)))
2×3 Matrix{CartesianIndex{2}}:
 CartesianIndex(1, 1)  CartesianIndex(1, 2)  CartesianIndex(1, 3)
 CartesianIndex(2, 1)  CartesianIndex(2, 2)  CartesianIndex(2, 3)

(BlockArrays) julia> collect(BlockRange((2,3)))
2×3 Matrix{Block{2, Int64}}:
 Block(1, 1)  Block(1, 2)  Block(1, 3)
 Block(2, 1)  Block(2, 2)  Block(2, 3)

@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Merging #268 (06d4150) into master (e359943) will increase coverage by 0.11%.
The diff coverage is 96.55%.

@@            Coverage Diff             @@
##           master     #268      +/-   ##
==========================================
+ Coverage   91.78%   91.89%   +0.11%     
==========================================
  Files          16       16              
  Lines        1461     1481      +20     
==========================================
+ Hits         1341     1361      +20     
  Misses        120      120              
Impacted Files Coverage Δ
src/BlockArrays.jl 66.66% <ø> (ø)
src/blockindices.jl 82.63% <90.90%> (+0.11%) ⬆️
src/abstractblockarray.jl 93.58% <100.00%> (+0.16%) ⬆️
src/blockaxis.jl 96.26% <100.00%> (ø)
src/blocklinalg.jl 94.57% <100.00%> (+0.73%) ⬆️
src/pseudo_blockarray.jl 86.84% <100.00%> (+0.11%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jishnub jishnub merged commit d8f82ad into JuliaArrays:master Jun 22, 2023
@jishnub jishnub deleted the viewblock1 branch June 22, 2023 05:05
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.

1 participant