v1.0

[dlfivefifty]

• Compact show for BlockRange

• update docstrings

• don't specialize show for zero dim

• fix missing io in print

• missing show tests

• show for BlockIndexRange

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 94.78908% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 95.50%. Comparing base (ee08389) to head (70e6e02). Report is 11 commits behind head on master.

Files	Patch %	Lines
src/blockedarray.jl	87.50%	14 Missing :warning:
src/blockaxis.jl	97.05%	3 Missing :warning:
src/blockbroadcast.jl	90.90%	2 Missing :warning:
src/blockcholesky.jl	0.00%	1 Missing :warning:
src/views.jl	50.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #255      +/-   ##
==========================================
+ Coverage   94.53%   95.50%   +0.97%     
==========================================
  Files          16       18       +2     
  Lines        1501     1625     +124     
==========================================
+ Hits         1419     1552     +133     
+ Misses         82       73       -9     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[dlfivefifty]

@jishnub I think the LazyArrays.jl extension should probably live in LazyArrays.jl.

Partly because a lot of LazyBandedMatrices.jl should be moved there. It'll be a lot of work to move that over...

[dlfivefifty]

@jishnub do you know what's going on with the CI?

[jishnub]

Do you mean why the checks are being skipped? I've used a plugin to skip duplicate checks if a commit has already been tested.

[dlfivefifty]

@jishnub @mtfishman I have essentially all my downstream packages working. I think this is ready to merge and tag. Anything else you wanted merged in first?

[mtfishman]

The only potentially breaking change I would maybe want to see is changing the definition of blocksizes as discussed in #369. I would like to see some discussion of that, I'm basically convinced that the current definition doesn't make much sense and isn't very useful, and suggested a more useful definition in #369.

I'm also not entirely convinced of the current slicing behavior of BlockArrays/AbstractBlockedUnitRange when they are sliced with UnitRange, i.e. #347, but I see the argument from @jishnub that preserving the axes of the indices used in the slicing operation is more consistent with conventions in other parts of the Julia ecosystem (besides slicing with Colon) and the functionality I was hoping for can be added later with additional syntax like @blocked A[1:3, 1:3].

[mtfishman]

Not sure if #373 will also be considered for part of the v1 release?

[dlfivefifty]

What was your final suggestion for the syntax for that?

[mtfishman]

Basically that we change the definition of blocksizes so that it acts like this:

julia> using BlockArrays

julia> a = BlockArray(randn(5, 5), [2, 3], [2, 3])
2×2-blocked 5×5 BlockMatrix{Float64}:
  0.916747   1.45141   │   0.740126    0.663479  0.17119 
 -0.563452   0.574973  │   0.618916   -1.43895   1.40107 
 ──────────────────────┼─────────────────────────────────
  0.621363   2.86333   │   0.0611178  -0.928277  1.48074 
  0.106673  -0.673614  │  -1.49219     0.158793  0.564433
  0.017256  -1.10824   │   0.768847    0.923508  0.362625

julia> blocksizes(a)[1, 2]
(2, 3)

julia> blocksizes(a)
2×2 BlockSizes{2, BlockMatrix{Float64, Matrix{Matrix{Float64}}, Tuple{BlockedUnitRange{Vector{Int64}}, BlockedUnitRange{Vector{Int64}}}}}:
 (2, 2)  (2, 3)
 (3, 2)  (3, 3)

i.e. it acts like a collection of size blocksize(a), and indexing into it outputs the size of the corresponding block.

The current definition gives:

julia> blocksizes(a)
([2, 3], [2, 3])

which is equivalent to:

julia> blocklengths.(axes(a))
([2, 3], [2, 3])

[dlfivefifty]

I found literally one usage of blocksizes in all my code (map(maximum, blocksizes(A))) so I think I have no strong opinion.

Though.... how would I do map(maximum, blocksizes(A)) after the change?

[mtfishman]

What's the goal of that operation?

[mtfishman]

If I understand correctly, the current definition of blocksizes(a) is equivalent to blocklengths.(axes(a)) so you could just do map(maximum, blocklengths.(axes(a))) or maximum.(blocklengths.(axes(a))) (I guess you want a tuple containing the maximum block length in each dimension?), which in my opinion is clearer anyway. I remember being very confused by the current definition of blocksizes when I was first using this package, and like you never really found use cases for it (besides the one you mentioned which can be replaced as I described).

[dlfivefifty]

Sounds good. I think blocksizes predates me redesigning the package to incorporate block structure in the axes.

[dlfivefifty]

What's the goal of that operation?

it finds the maximum block length. I think it’s used as a quick and dirty way to determine bandwidths from block bandwidths.. probably something that should be done in a smarter way anyways.

[mtfishman]

I'm also open to other suggestions for an interface for more conveniently obtaining the size of a block of a block array, I suggested a number of options in #369 but in the end my favorite interface was repurposing blocksizes, which I found particularly compelling since the definition I propose is a natural generalization of blocklengths.

[dlfivefifty]

OK I think that interface is reasonable.

In an ideal world we would have better names for the two concepts "tuple containing the number of blocks" (currently called blocksize) and "the size of specified block" (your proposed blocksizes). Along with the axes analogues. Unfortunately I can't think of better names. In the interest of not dragging things out (I'd like to get this tagged so the downstream changes can be merged) I would suggest we just implement your suggestion and leave any further renaming to v2.0.

[mtfishman]

I'm happy to make a PR with the new blocksizes definition, probably this week or next.

[dlfivefifty]

@jishnub @mtfishman Shall we merge and tag?

[jishnub]

Good with me

[mtfishman]

In an ideal world we would have better names for the two concepts "tuple containing the number of blocks" (currently called blocksize) and "the size of specified block" (your proposed blocksizes). Along with the axes analogues. Unfortunately I can't think of better names. In the interest of not dragging things out (I'd like to get this tagged so the downstream changes can be merged) I would suggest we just implement your suggestion and leave any further renaming to v2.0.

Agreed, I think it will be confusing for new users that blocksize and blocksizes have such similar names but have quite different meanings, and it feels very natural to want something like blocksize(a::Array, b::Block) for getting the size of a block. I also can't think of an alternative that isn't worse in some way or another, or at least not clearly better. My guess is that a new user's first guess for trying to get the equivalent of blocksize would be to look for a function called nblocks but I see why that wasn't chosen.

A more incremental renaming that could be beneficial for disambiguating blocksize and blocksizes could be to rename blocksize to blockedsize (and similarly blocklength to blockedlength and blockaxes to blockedaxes), any thoughts on that? Modulo that potential change, it seems like a good time to tag v1.0.

[dlfivefifty]

would be to look for a function called nblocks but I see why that wasn't chosen

I think it actually used to be called nblocks but I renamed it since that naming convention didn't exist elsewhere in Julia. I wanted to mimic the length/size/axes naming convention as much as possible.

A more incremental renaming that could be beneficial for disambiguating blocksize and blocksizes could be to rename blocksize to blockedsize (and similarly blocklength to blockedlength and blockaxes to blockedaxes), any thoughts on that? Modulo that potential change, it seems like a good time to tag v1.0.

I'd be open to it but I'm not convinced that it is better, or if it's better that it's enough better to merit rewriting a bunch of things and delaying the release. I'm also trying to decide whether the rhyming with blockedrange is appropriate.

My instinct is to leave things as-is since its not too hard to understand and the docs point to to the other functions.

[mtfishman]

Since I learned about BlockedUnitRange I actually was thinking that it would be nice to lean into the term "blocked" even more throughout the package, for example PseudoBlockArray could be renamed BlockedArray (which I think is a good name since it makes it clearer that it is an array that has been blocked, i.e. indicates more clearly that it is a wrapper around an array that adds on a blocking structure), and then blockedsize, blockedlength, etc. would be consistent with that naming scheme. But I could see why you might be hesitant to make that change at this point, and there would be a clear upgrade path to that kind of naming scheme in v2 or v3 since it doesn't overlap with current names.

[dlfivefifty]

Actually I like BlockedArray a lot. PseudoBlockArray was never a very good name and BlockedArray does mirror BlockedUnitRange rather nicely. I would vote for making that change in v1.0.

@jishnub any opinions on this?

[dlfivefifty]

Also @KristofferC please let us know if you have any opinions on changes as creator of this package!

[jishnub]

Is the proposal here to rename PseudoBlockArray to BlockedArray, and leave BlockArray as is?

[mtfishman]

Is the proposal here to rename PseudoBlockArray to BlockedArray, and leave BlockArray as is?

That's what I had in mind.

[jishnub]

The change makes sense to me, although I wonder if the two names being so similar might lead to confusion?

[dlfivefifty]

Everything leads to confusion 🤷‍♂️ PseudoBlockArray definitely leads to confusion.

[jishnub]

Alright, I don't have issues with this change, so perhaps we should go ahead

[mtfishman]

I made #400 to track this issue. I'm happy to make a PR this week.