Don't materialize an `AdjointTensorMap` in `TO.tensoradd!` when `conjA = true` by leburgel · Pull Request #520 · QuantumKitHub/TensorKit.jl

leburgel · 2026-08-27T14:28:35Z

A second option for a proper fix for #516, which avoids materializing an AdjointTensorMap in TO.tensoradd! altogether, but rather passes the conj flag along to the array-level tensoradd!

Basically the same underlying idea as #519, but since I don't exactly know what I'm doing here I'm posting both, mostly just for inspiration.

Some benchmark timings based on the reproducer of #516:

sector type	old	patched	plain TensorMap	old ratio	patched ratio	speedup
fℤ₂ ⊠ U(1)	0.1245 ms	0.0375 ms	0.0336 ms	3.70×	1.12×	3.32×
SU(2)	2.8247 ms	0.3739 ms	0.3448 ms	8.19×	1.08×	7.55×
fℤ₂ ⊠ U(1) ⊠ SU(3)	4.0868 ms	0.0510 ms	0.0512 ms	79.80×	1.00×	80.08×

codecov · 2026-08-27T15:53:04Z

Codecov Report

❌ Patch coverage is 89.28571% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/tensors/indexmanipulations.jl	83.33%	1 Missing ⚠️
src/tensors/tensoroperations.jl	85.71%	1 Missing ⚠️
src/tensors/treetransformers.jl	85.71%	1 Missing ⚠️

Files with missing lines	Coverage Δ
src/spaces/homspace.jl	`91.83% <100.00%> (+0.46%)`	⬆️
src/tensors/indexmanipulations.jl	`88.23% <83.33%> (-0.12%)`	⬇️
src/tensors/tensoroperations.jl	`97.00% <85.71%> (-0.38%)`	⬇️
src/tensors/treetransformers.jl	`82.07% <85.71%> (-0.45%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

lkdvos

Overall this looks great, it's definitely the right call to also specialize the adjointtensor, I've just been too lazy and putting it off, hoping that the actual runtime fraction is not too high. I somehow didn't realize that the structure mapping in the current state is actually not that complicated, so this does indeed open up many optimization possibilities.

On a larger scale comment, I'm not sure if this is really a route we want to take, but it does look like we could conceivably reduce the cache sizes by simply making the "treetransformers" only store which tokens go to which other tokens, and therefore decouple that from the actual subblockstructure dictionaries that can then fill out which data is where. This might be a bit larger of a refactor though, so I'll see what I can do next week and if this really makes sense

lkdvos · 2026-08-28T10:58:55Z

+        tdst, tsrc, p, transformer, α, β, backend, allocator, scheduler;
+        conjsrc::Bool = false


Suggested change

tdst, tsrc, p, transformer, α, β, backend, allocator, scheduler;

conjsrc::Bool = false

tdst, tsrc, p, conjsrc, transformer, α, β, backend, allocator, scheduler

little bit of a nitpick, but I'd be inclined to just make this a mandatory (positional) argument instead, which is more in line with the remainder of these functions. Obviously doens't matter, just for consistency.

lkdvos · 2026-08-28T11:04:53Z

-        A′ = adjoint(A)
        pA′ = adjointtensorindices(A, _canonicalize(pA, C))
-        permute!(C, A′, pA′, α, β, backend, allocator)
+        if C isa TensorMap && A isa TensorMap


It somehow feels like this specialization might make more sense on the permute!-level directly, where we could intercept TensorMap-AdjointTensorMap combinations directly? That would also immediately catch manual calls to permute!(tdst, tsrc', ...) in the same way.

To achieve this, I'm wondering if it makes sense to just have treebraider(Vdst, Vsrc, p, levels, conj) as the entrypoint, which then also fixes the caching keys being distinct? There's some subtleties left with this, mostly with respect to other combinations like permute!(tdst', tsrc, ...), which we would then probably want to map to the other case, but that might be reasonable?

I think the first part of this comment is more similar to what is sketched in #519, as opposed to the tensoradd!-level fix that is sketched here.

lkdvos · 2026-08-28T11:06:00Z

+
+See also [`subblockstructure`](@ref).
+"""
+function adjoint_subblockstructure(W::HomSpace)


code-organization wise, would it be easier to make this subblockstructure(W::HomSpace, conjW::Bool=false)?

lkdvos · 2026-08-28T11:08:33Z

+    newkeys = map(((f₁, f₂),) -> (f₂, f₁), collect(keys(structs)))
+    newvals = map(collect(values(structs))) do (sz, str, off)


it feels like it should be possible to avoid the collect calls here, but I'd have to dig through the Dictionaries interface(s) to see what is possible?

Index manipulations now run through a single kernel that operates on subblocks addressed by position: `StridedSubblocks` (sector-independent views into the flat data of a `TensorMap`) or `TreeSubblocks` (any `AbstractTensorMap`, through `subblock`), both carrying an optional lazy conjugation. `TreeTransformer`s store only the mapping between subblock positions and recoupling coefficients, alongside the subblock structures, and are cached for every tensor type. Adjoint sources and destinations, as well as `conj` in `tensoradd!`, are folded into a conjugation flag, relabeled permutation and levels, and conjugated scalars, so that `AdjointTensorMap` wrappers no longer force the uncached generic path (fixes #516, supersedes #519 and #520). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

lkdvos · 2026-09-09T12:55:16Z

Closing this in favor of #526

Index manipulations now run through a single kernel that operates on subblocks addressed by position: `StridedSubblocks` (sector-independent views into the flat data of a `TensorMap`) or `TreeSubblocks` (any `AbstractTensorMap`, through `subblock`), both carrying an optional lazy conjugation. `TreeTransformer`s store only the mapping between subblock positions and recoupling coefficients, alongside the subblock structures, and are cached for every tensor type. Adjoint sources and destinations, as well as `conj` in `tensoradd!`, are folded into a conjugation flag, relabeled permutation and levels, and conjugated scalars, so that `AdjointTensorMap` wrappers no longer force the uncached generic path (fixes #516, supersedes #519 and #520). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…#526) * Refactor index manipulation kernels around position-indexed subblocks Index manipulations now run through a single kernel that operates on subblocks addressed by position: `StridedSubblocks` (sector-independent views into the flat data of a `TensorMap`) or `TreeSubblocks` (any `AbstractTensorMap`, through `subblock`), both carrying an optional lazy conjugation. `TreeTransformer`s store only the mapping between subblock positions and recoupling coefficients, alongside the subblock structures, and are cached for every tensor type. Adjoint sources and destinations, as well as `conj` in `tensoradd!`, are folded into a conjugation flag, relabeled permutation and levels, and conjugated scalars, so that `AdjointTensorMap` wrappers no longer force the uncached generic path (fixes #516, supersedes #519 and #520). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * blockiterator->blockiterators * clean up trivial symmetry bypassing overhead * Address review: normalize subblock parent, prime conjsrc `StridedSubblocks` now stores its data the way `StridedView` parents it (an `Array` becomes its underlying `Memory` on Julia >= 1.11), asking `StridedView` itself rather than reproducing that rule. This makes the view type a direct function of the type parameters, so `eltype` can be written out instead of going through `Core.Compiler.return_type`. As a consequence `storagetype` reports the normalized type, which is not an `Array`, so the CPU branch of `_adapt_recoupling` is keyed on a `CPUStorage` alias to keep the recoupling matrices off the `Adapt` path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Test fix: pick a multifusion-compatible space for the isometry test `V1 ⊗ V2 ← V3 ⊗ V4` does not close the unit cycle that `GenericUnit` sectors require, so constructing it threw a `SpaceMismatch` for the `IsingBimodule` space lists. The sibling `Permutations: adjoint operands` testset never hit this because it sits behind `symmetricbraiding`, which is false for multifusion; this one builds its space unconditionally. `V1 ⊗ V5 ← V2 ⊗ V4` does close the cycle, and is equally general for the other sectors, so use that rather than skipping multifusion: the transpose half of the test then covers them too, while the braid half stays behind `hasbraiding` (multifusion is `NoBraiding`). Only Windows and macOS saw this, since `default_spacelist` hands out different space lists per OS on CI and only those two include the multifusion entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * reorganize to fix docstring * simplify getting number of transformer_threads * mark `add_transform` as non-public * fix (unrelated) docstring sentence * all has_array_view * clamp -> min * docstring improvements * simplify treetransformer implementations * Rename `AbelianTreeTransformer` to `UniqueTreeTransformer` "Abelian" is ambiguous for sectors: it can refer either to the fusion of two sectors having a unique result, or to the commutativity of the fusion rules. The transformer is selected on `FusionStyle(I) == UniqueFusion()`, so name it after that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Pass conjugation as a flag instead of a subblock view op `StridedSubblocks` and `TreeSubblocks` no longer apply `identity`/`conj` to every view. Instead `conjsrc` is threaded through `add_transform_kernel!` into `_add_transform_block!`, where it is handed to `TO.tensoradd!` as its `conjA` argument, at the single-tree call and when packing a multi-tree block. This drops a type parameter from both collections, so the kernel compiles to one instance per (storage, numind) rather than one per conjugation. The runtime flag is free: `flag2op` is union-split, and `conj` of a real-eltype `StridedView` is a type-level no-op, which also makes the previous `scalartype(t) <: Real` guard redundant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * remove TreeSubblocks and route through StridedSubblocks --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

Attempt at speeding up permutes of AdjointTensorMaps

23c18ce

leburgel changed the title ~~Don't materialize an AdjointTensorMap in `~~ Don't materialize an AdjointTensorMap in TO.tensoradd! when conjA = true Aug 27, 2026

leburgel mentioned this pull request Aug 27, 2026

Permuting an AdjointTensorMap seems much slower than it should be #516

Closed

lkdvos reviewed Aug 28, 2026

View reviewed changes

lkdvos mentioned this pull request Sep 3, 2026

Refactor index manipulation kernels around position-indexed subblocks #526

Merged

lkdvos closed this Sep 9, 2026

Navigation Menu

Don't materialize an `AdjointTensorMap` in `TO.tensoradd!` when `conjA = true` - #520

Don't materialize an `AdjointTensorMap` in `TO.tensoradd!` when `conjA = true`#520
leburgel wants to merge 1 commit into
mainfrom
lb/conj_threading

leburgel commented Aug 27, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 27, 2026 •

edited

Loading

Uh oh!

lkdvos left a comment

Uh oh!

lkdvos Aug 28, 2026

Uh oh!

lkdvos Aug 28, 2026

Uh oh!

leburgel Aug 30, 2026

Uh oh!

lkdvos Aug 28, 2026

Uh oh!

lkdvos Aug 28, 2026

Uh oh!

lkdvos commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		tdst, tsrc, p, transformer, α, β, backend, allocator, scheduler;
		conjsrc::Bool = false

	tdst, tsrc, p, transformer, α, β, backend, allocator, scheduler;
	conjsrc::Bool = false
	tdst, tsrc, p, conjsrc, transformer, α, β, backend, allocator, scheduler

		newkeys = map(((f₁, f₂),) -> (f₂, f₁), collect(keys(structs)))
		newvals = map(collect(values(structs))) do (sz, str, off)

Conversation

leburgel commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lkdvos left a comment

Choose a reason for hiding this comment

Uh oh!

lkdvos Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

lkdvos Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

leburgel Aug 30, 2026

Choose a reason for hiding this comment

Uh oh!

lkdvos Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

lkdvos Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

lkdvos commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

leburgel commented Aug 27, 2026 •

edited

Loading

codecov Bot commented Aug 27, 2026 •

edited

Loading