Restore inferrable return type for block tensor slicing - #73
Open
lkdvos wants to merge 1 commit into
Open
Conversation
`_slice_getindex` chose between returning a single block and a sliced block tensor with a
runtime `inds isa NTuple{M, Int}` check, and `_slice_getindex_single` reached that check with
an index tuple of element type `Union{Int, UnitRange{Int}}`, so the check could not be
resolved statically. `blocktensor[2:(end - 1)]` on a tensor with a single nontrivial
dimension therefore inferred as `Union{BlockTensorMap{...}, TensorMap{...}}` rather than
`BlockTensorMap{...}`, and a logical mask inferred as `Any`.
Make the choice by dispatch instead, and fill the trivial dimensions with `1:1` so that `Int`
stays out of the index tuple. A mask is normalized up front, since it may consume several
dimensions and `Base.to_indices` cannot infer one mixed with the fillers.
Runtime behaviour is unchanged: slicing was verified to give identical types and values to
v0.3.16 across every index form (`Int`, ranges, step ranges, `Colon`, index vectors, logical
masks; single-index and full-rank) on dense and sparse tensors of one and three dimensions.
Downstream this widened MPSKit's Jordan-form derivative operators to their abstract type,
which in turn made `excitations` with `ChepigaAnsatz`/`ChepigaAnsatz2` uninferrable.
The new tests cover the single-index path on a multi-index tensor, which had none: the
existing single-index tests use a genuinely one-dimensional tensor and so take the
`M == numind(t)` fast path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a type-inference regression introduced in v0.3.16 by #72, and bumps the version to v0.3.17.
The regression
_slice_getindexchose between returning a single block and a sliced block tensor with a runtimeinds isa NTuple{M, Int}check, and_slice_getindex_singlereached that check with an index tuple built asntuple(i -> i == d ? only(indices) : 1, numind(t))— element typeUnion{Int, UnitRange{Int}}, so the check could not be resolved statically and the union leaked into the return type.On an MPS-environment-shaped tensor (size
(1, 3, 1)):t[2:(end - 1)]inferred asBlockTensorMap{…}Union{BlockTensorMap{…}, TensorMap{…}}The same applied to
t[:],t[1:2:3]andt[[1, 3]]; a logical mask (t[[true, false, true]]) inferred asAny, for a second reason — a mask may consume several dimensions, soBase.to_indicescannot infer one mixed with the1fillers.The fix
Make the single-block-vs-slice choice by dispatch (
_slice_getindex_full), and fill the trivial dimensions with1:1rather than1soIntstays out of the index tuple. Masks are normalized to an index vector before being placed.After the fix, every index form on every shape infers concretely —
Int,UnitRange,StepRange,Colon, index vector and logical mask, single-index and full-rank, dense and sparse.Why it matters
MPSKit's Jordan-form derivative operators store
O3 = typeof(A)whereAis built fromGL[2:(end - 1)], so the widened slice type madeAC_hamiltonianinfer as the bare abstractJordanMPO_AC_Hamiltonian, and from thereexcitationswithChepigaAnsatz/ChepigaAnsatz2inferredTuple{Any, Vector}— a CI failure in MPSKit's@testinferredtests. There is no measurable runtime cost (DMRG, 20 sites, χ=32, 5 sweeps: 0.353 s on v0.3.15 vs 0.363 s on v0.3.16), so this is purely a type-stability contract regression.Verification
excitations"finite" testset passes 25/25 (it was 21 pass / 4 fail), on both Julia 1.10.11 and 1.12.6.Tests
test/abstracttensor/indexing.jlgains:single-index slicingtestset on a multi-index tensor with one nontrivial dimension. The existing single-index tests use a genuinely one-dimensional tensor, which takes theM == numind(t)fast path, so this path had no coverage at all.indexing inferencesweep asserting a concrete return type for every index form across six tensor shapes, so a future rewrite of the index handling is caught for all of them rather than the hand-picked cases.Both fail on v0.3.16 (the sweep flags 5 of the 6 single-index forms).
🤖 Generated with Claude Code