Skip to content

Intercept TO.tensoradd! instead of TensorKit.add_transform! - #78

Merged
lkdvos merged 2 commits into
mainfrom
ld-intercept-tensoradd
Sep 15, 2026
Merged

lkdvos merged 2 commits into
mainfrom
ld-intercept-tensoradd

Conversation

@lkdvos

@lkdvos lkdvos commented Sep 15, 2026

Copy link
Copy Markdown
Member

Problem

The blockwise tensoradd implementations here were only reachable through TensorKit internals:

  • TO.tensoradd! delegated to permute!, which this package overrides, and
  • the TK.add_transform! methods in src/tensors/indexmanipulations.jl caught the kernel.

Neither is a contract TensorKit owes us, and both stopped holding on TensorKit's
index-manipulation refactor (TensorKit.jl#526),
where TO.tensoradd! calls the braid kernel directly and add_transform! gained a conjsrc
argument (8 → 9 positional arguments, so all five of our methods silently became unreachable).

Nothing errors in that state. Dense block tensors survive via TensorKit's generic subblock
fallback, but sparse block tensors silently produce zeros, because the fallback writes through
blocks a sparse container never materialized:

W = Vtr[1] ⊗ Vtr[2] ⊗ Vtr[3] ← Vtr[4] ⊗ Vtr[5]
A = sprand(Float32, W, 0.5)
@tensor C[4, 5, 1, 3, 2] := A[1, 2, 3, 4, 5]   # norm(C) == 0

Measured against TensorKit#526, test/linalg/tensoroperations.jl fails, and TO.tensoradd! on a
SumSpace(ℂ^8, ℂ^8, ℂ^8) 4-leg block tensor also regresses 262 µs → 10473 µs.

Fix

Implement TO.tensoradd! for the block tensor types. That is the public entry point @tensor
lowers to, so it stays reachable however TensorKit arranges its kernels, and conjA is handled
explicitly rather than relying on it having been unwrapped into an adjoint beforehand.

The mixed block/plain methods take the concrete TensorMap so that
(BlockTensorMap, SparseBlockTensorMap) and the reverse resolve to the general method instead of
being ambiguous.

The five TK.add_transform! methods are left in place: they are still live against released
TensorKit's permute! routing, and our compat is TensorKit = "0.17". They can go when that
bound moves.

Tests

Two new testsets, both calling the entry points directly rather than through @tensor, since
the existing tests only exercised the macro and so pinned none of them:

  • tensoradd! entry point — TO.tensoradd! over sparse/dense × two permutations × conjA.
  • planar entry points — planaradd!, plus @planar trace and contraction. These are not
    currently broken (planaradd! → transpose!, planartrace! → trace_permute!,
    planarcontract! → contract! all still land on methods we override), but they rest on the same
    undocumented delegation, and there was previously no planar coverage here at all.

Verification

TO.tensoradd! @tensor permute ambiguities tests
TensorKit 0.17.1, before 260 µs 779 µs 0 pass
TensorKit#526, before 10273 µs 10712 µs 0 fail (zeros)
TensorKit 0.17.1, after 268 µs 769 µs 0 pass
TensorKit#526, after 269 µs 787 µs 0 pass

Aqua clean (including the method-ambiguity check) and runic clean.

🤖 Generated with Claude Code

lkdvos and others added 2 commits September 15, 2026 14:28
The blockwise `tensoradd` implementations were only reachable through
`TensorKit`'s internals: `TO.tensoradd!` delegated to `permute!`, and the
`add_transform!` methods defined here caught the kernel. Neither is a
contract `TensorKit` owes us, and both stopped holding on TensorKit's
index-manipulation refactor (QuantumKitHub/TensorKit.jl#526), where
`TO.tensoradd!` calls the braid kernel directly and `add_transform!`
gained a `conjsrc` argument. Nothing errors in that case: dense block
tensors survive via TensorKit's generic subblock fallback, while sparse
ones silently produce zeros, because the fallback writes through blocks a
sparse container never materialized.

Implement `TO.tensoradd!` for the block tensor types instead. That is the
public entry point `@tensor` lowers to, so it is reachable no matter how
TensorKit arranges its kernels, and `conjA` is handled explicitly rather
than relying on it having been unwrapped into an adjoint beforehand.

The mixed block/plain methods take the concrete `TensorMap` so that
`(BlockTensorMap, SparseBlockTensorMap)` and the reverse resolve to the
general method rather than being ambiguous.

The added testset calls `TO.tensoradd!` directly rather than through
`@tensor`, so this stays covered independently of TensorKit's routing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`planaradd!`, `planartrace!` and `planarcontract!` are reached through
`transpose!`, `trace_permute!` and `contract!`, so they currently land on
methods defined here and are not affected by TensorKit#526. They rest on
the same undocumented delegation that broke `tensoradd!` though, and there
was no planar coverage here at all, so pin them directly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.18182% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/tensors/tensoroperations.jl 68.18% 7 Missing ⚠️
Files with missing lines Coverage Δ
src/tensors/tensoroperations.jl 85.22% <68.18%> (-5.69%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lkdvos
lkdvos merged commit 2a04bdc into main Sep 15, 2026
25 of 28 checks passed
@lkdvos
lkdvos deleted the ld-intercept-tensoradd branch September 15, 2026 20:30
@lkdvos lkdvos mentioned this pull request Sep 23, 2026
lkdvos referenced this pull request Sep 23, 2026
* Bump version to v0.3.19

* Adapt multifusion `SumSpace` to TensorKit v0.17.2 coloring rules

TensorKit v0.17.2 (#515) requires every `GradedSpace` to be homogeneously
colored, forbids `unitspace` for `GenericUnit` sectors and checks coloring
when building `ProductSpace`/`HomSpace`. This broke the multifusion `SumSpace`
tests, which flattened heterogeneous sums into a single `GradedSpace`.

- `unitspace(::Type{<:SumSpace})` returns one component per simple unit
- `leftunitspace`/`rightunitspace` inspect components instead of flattening
- `_leftrightunit(::SumSpace)` uses the shared unit, or a wildcard if components differ
- tests use `⊞` instead of flattening `⊕`, and expect incompatible products to throw
- require TensorKit v0.17.2

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Require multifusion SumSpace to be homogeneously colored as a whole

The previous fix let leftunitspace/rightunitspace answer for a SumSpace
whose components agreed on only one side (left xor right), by checking
each side independently and treating disagreement on the other side as
a wildcard. That's inconsistent with how a plain GradedSpace works: it
is either homogeneously colored (single left AND right unit shared by
every sector) or invalid. A SumSpace should follow the same rule as a
whole, so leftunitspace/rightunitspace/_leftrightunit again require
full agreement on both sides and reject (SpaceMismatch) otherwise,
without any special-casing for a partial match.

unitspace itself is unaffected by this: TensorKit already forbids it at
the type level for GenericUnit sector types, regardless of any given
instance's homogeneity, so it throws ArgumentError unconditionally for
IsingBimodule-sectored spaces, homogeneous or not. The Multifusion
testset is adjusted to expect this throughout, rather than the previous
attempt to give unitspace a real answer for such sector types.

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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