Preserve block information in more slicing operations #459
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #459 +/- ##
==========================================
+ Coverage 93.64% 93.66% +0.02%
==========================================
Files 19 19
Lines 1667 1673 +6
==========================================
+ Hits 1561 1567 +6
Misses 106 106 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I assume I'll have to add more methods for |
src/blockindices.jl
Outdated
|
|
||
| Block(bs::BlockSlice{<:BlockIndexRange}) = Block(bs.block) | ||
|
|
||
| struct BlockInds{BB,T<:Integer,INDS<:AbstractVector{T}} <: AbstractVector{T} |
There was a problem hiding this comment.
Can you add a docstring so I know what this is meant to do?
There was a problem hiding this comment.
Can do. I was holding off doing "final touches" like that to get your reaction if this is something you want in the first place (and get feedback on the name).
There was a problem hiding this comment.
I can't react to something I don't understand 😅
There was a problem hiding this comment.
Gotcha, I thought I described it in the first comment of the PR but I can add a docstring as well.
There was a problem hiding this comment.
I've added a docstring along with some tests, let me know if that helps to clarify the purpose of this object. I changed the name to BlockedSlice but I'm not so happy with that and again I'm open to suggestions.
This PR is split off from #462, it renames `BlockedSlice` introduced in #459 to `NoncontiguousBlockSlice` and also adds more functionality and tests so that it is as featureful as `BlockSlice`. The reason for the name change is that in #462, this type will be used in slicing operations such as `V[[Block(1), Block(3)]]` and `V[Block(1)[[1, 3]]]`. In my opinion, `BlockedSlice` doesn't make a lot of sense as a name for the latter case. The more general concept is that it is a slice object constructed when there is a non-contiguous blockwise slice of some form (either non-contiguous blocks or non-contiguous within a block). I'm open to other name suggestions, `NoncontiguousBlockSlice` is the best I could come up with and there isn't really an analogous case for this in Base from what I've seen. Note the main reason why we can't just use `BlockSlice` for those cases is that `BlockSlice` is an `AbstractUnitRange` subtype and `NoncontiguousBlockSlice` covers cases where the slices aren't ranges, so maybe `BlockSliceVector` could work as a name to emphasize that point. --------- Co-authored-by: Sheehan Olver <[email protected]>
2 participants
This introduces a generalization of
BlockSlicefor views involving vectors ofBlockandBlockIndexRange, building off of #445.I call it
BlockedSlicebut I'm open to suggestions. At first I tried to just generalizeBlockSliceto be anAbstractVectorsubtype but some operations depend on it being anAbstractUnitRangeso I think it makes sense to have both.The motivation is that in https://github.com/ITensor/BlockSparseArrays.jl it would be helpful to preserve information about the original blocks being sliced when taking views involving vectors of
BlockandBlockIndexRange. #445 drops the blocks input in the slice operation, so it is hard to tell if a certain slice operation was really originally blockwise.@dlfivefifty curious to hear your thoughts on this since you wrote #445.