Skip to content

Fix planar index reordering and remove _contractedspace - #532

Merged
lkdvos merged 6 commits into
mainfrom
lb/planar-general-pab
Sep 20, 2026
Merged

lkdvos merged 6 commits into
mainfrom
lb/planar-general-pab

Conversation

@lkdvos

@lkdvos lkdvos commented Sep 11, 2026 •

Copy link
Copy Markdown
Member

Fixes the root cause that #531 works around, and supersedes it.

Problem

@planar allocates through TensorOperations.tensoralloc_contract with the raw index tuples of the planar decomposition, in which pA and pB need not be planar partitions by themselves — only planarcontract! rotated them into shape, and only when pAB could be absorbed into those rotations. Since #515 added the coloring checks to the ProductSpace/HomSpace constructors, computing the output space as permute(compose(permute(VA, pA), permute(VB, pB)), pAB) throws for GenericUnit sectors, which is why #515 replaced it by the leg-wise _contractedspace. That workaround pins both operands to a single spacetype, which breaks contracting operands whose spacetype parameters differ (a BlockTensorMap with a TensorMap, as reported from MPSKit in #531); the fallback added there fixes the MethodError but reintroduces exactly the intermediate spaces that #515 removed.

Changes

  • planarcontract! takes an arbitrary cyclic pAB, like the non-planar contract!: a residual permutation is applied with transpose! through an intermediate, mirroring what blas_contract! does with permute!. The remaining difference between planar and non-planar contraction is transpose vs permute, which is also what the AD rules of Planar additions [WIP] #124 need.
  • reorder_indices is replaced by planar_contract_indices(A, pA, B, pB, pAB) -> pA′, pB′, pAB′: it rotates each operand's index cycle so the contracted indices form a contiguous arc and remaps pAB instead of absorbing it. It is shared with the two BraidingTensor methods and works on HomSpaces as well as on tensors.
  • @planar canonicalizes the index tuples it emits at macro-expansion time: planar_contract_indices only needs the index partitions of both operands, and those are written in the expression and already checked against the actual tensors by _extract_tensormap_objects. Both the contraction and its allocation therefore see planar partitions, and the allocation needs no planar-specific entry point. Hand-written planar kernels should canonicalize with planar_contract_indices before allocating with tensoralloc_contract; planarcontract! keeps canonicalizing at runtime, so the kernel itself is safe either way.
  • ⊗ allocates its destination directly; its pA = ((all indices), ()) intermediate is non-planar by construction.
  • tensorcontract_structure composes the intermediate spaces again and _contractedspace is gone, which restores mixed spacetype support. For GenericUnit sectors, pA and pB passed to it must now be planar partitions.

Tests

New multifusion coverage (there was no @planar binary contraction on multifusion spaces): a planar contraction whose partitions are non-planar by themselves, ⊗ with more than one index in the domain, a non-trivial and a non-cyclic pAB, planar_contract_indices unit tests including the fully contracted and non-planar cases, and a check on the index tuples of the macro expansion.

Verified on the full space list for test/tensors/{contractions,braidingtensor,planar,linalg,indexmanipulations}.jl and test/symmetries/spaces.jl. The mixed-spacetype case has no in-repo coverage (it needs a downstream space type), so it is worth a run against BlockTensorKit + MPSKit before merging.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.75281% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/planar/planaroperations.jl 95.55% 2 Missing ⚠️
Files with missing lines Coverage Δ
src/planar/macros.jl 84.94% <100.00%> (+0.50%) ⬆️
src/planar/postprocessors.jl 100.00% <100.00%> (ø)
src/planar/preprocessors.jl 88.43% <100.00%> (+0.21%) ⬆️
src/spaces/homspace.jl 92.25% <100.00%> (-0.43%) ⬇️
src/tensors/braidingtensor.jl 88.05% <100.00%> (+0.46%) ⬆️
src/tensors/linalg.jl 83.23% <100.00%> (+0.30%) ⬆️
src/tensors/tensoroperations.jl 96.44% <100.00%> (ø)
src/planar/planaroperations.jl 70.31% <95.55%> (-2.88%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leburgel

Copy link
Copy Markdown
Member

Always happy to be superseded, this looks great!

@lkdvos
lkdvos force-pushed the lb/planar-general-pab branch from 095fcbe to 8ba2eaa Compare September 15, 2026 17:27
Comment thread docs/src/man/contractions.md Outdated
Comment thread src/planar/planaroperations.jl
Comment thread src/planar/planaroperations.jl Outdated
@lkdvos
lkdvos force-pushed the lb/planar-general-pab branch from 8ba2eaa to acf2daf Compare September 17, 2026 17:59
Comment thread src/planar/macros.jl Outdated
Comment thread test/tensors/contractions.jl

@Jutho Jutho left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to small suggestions, very much approved.

lkdvos and others added 6 commits September 20, 2026 09:54
…pace`

`@planar` allocates through `TO.tensoralloc_contract` with the raw index
tuples of the planar decomposition, in which `pA` and `pB` need not be planar
partitions by themselves; only `planarcontract!` rotated them into shape, and
only when `pAB` could be absorbed into those rotations.

Replace `reorder_indices` by `planar_contract_indices`, which canonicalizes
`pA` and `pB` and remaps `pAB` instead of absorbing it, let `planarcontract!`
apply a residual cyclic `pAB` with `transpose!` through an intermediate, and
allocate planar destinations through the new `planaralloc_contract`. Allocate
the destination of `⊗` directly, since its intermediates are non-planar by
construction.

With every caller passing planar partitions, `tensorcontract_structure` can
again compose the intermediate spaces, which also restores contractions of
operands with different `spacetype` parameters.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`planar_contract_indices` only needs the index partitions of both operands, which
`@planar` knows: `_extract_tensormap_objects` checks the partitions written in the
expression against the actual tensors. Record them while preprocessing and
canonicalize the index tuples of the emitted planar contractions, so that the
runtime canonicalization is left with nothing to do for macro-generated code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Now that `@planar` emits canonical index tuples, its allocations no longer need
to be canonicalized at runtime and can go through `tensoralloc_contract` again.
Canonicalizing before the planar operations are inserted also means that the
contraction and its allocation still share a single argument layout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rename `_index_partitions!` to `_record_index_partitions!` so the
preprocessor name starts with a verb, and take `ex` as its first argument
for consistency with the other preprocessors. Add the missing `!` to
`_decompose_planar_contractions!`, which likewise mutates its accumulator.

Also test the planar contraction with explicit parentheses, so that both
association orders of the three-factor contraction are covered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`mul!(tC, tA, tB, α, β)` forwarded `VectorInterface.One`/`Zero` verbatim
to LinearAlgebra's block `mul!`. Ordinary GEMM tolerates this, but a
contraction of a tensor with its own adjoint dispatches to the HERK path,
where `herk_wrapper!` calls `isreal` on the scalars and throws a
`MethodError`. Map them to `true`/`false` first, which LinearAlgebra still
recognizes as exact one and zero.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lkdvos
lkdvos force-pushed the lb/planar-general-pab branch from acf2daf to bb637c8 Compare September 20, 2026 14:39
@lkdvos
lkdvos merged commit 2f82611 into main Sep 20, 2026
67 checks passed
@lkdvos
lkdvos deleted the lb/planar-general-pab branch September 20, 2026 17:23
lkdvos added a commit that referenced this pull request Sep 21, 2026
* Draft changelog for v0.17.2

Consolidates the Unreleased section (which already included the real
entries added by #526/#532 on merge) with entries for the remaining
PRs merged since v0.17.1 (#487-#535), and retitles it as 0.17.2.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Bump version to v0.17.2

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix confirmed small bugs from the pre-release audit

- isunitspace: require dim(V) == 1 for GenericUnit sectors (#537)
- GradedSpace ⊕/supremum: check unit homogeneity of the result (#538)
- isconj(::ComplexSpace): return isdual(V) instead of always true (#539)
- multi_associator: return a vector, not a scalar, on early-exit for
  GenericFusion (#540)
- split(f, 0): use leftunit(f.coupled) instead of indexing an empty
  uncoupled tuple (#541)
- repartition: return a Pair in the identity branch, matching every
  other branch (#542)
- Mooncake scalar_pullback: accumulate into the tangent instead of
  overwriting it (#543)
- rand/randn/randexp/randisometry(rng, T, space): fix one(domain) typo (#544)
- pinv(::DiagonalTensorMap): fix inverted atol/rtol defaulting and
  empty-tensor throw (#545)
- t1 / t2: promote to a float scalartype, matching t1 \ t2 (#546)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Add changelog entry for the audit bugfixes

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address fable review findings on the audit bugfixes

- split(f, 0): also guard innerlines_extended construction, which
  still indexed the empty uncoupled tuple for a 0-leg tree
- pinv(::DiagonalTensorMap): use eps (not sqrt(eps)) for the default
  rtol, matching _default_rtol's convention and dense LinearAlgebra.pinv

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address tuicr review comments on the audit bugfixes

- pinv(::DiagonalTensorMap): reuse _default_rtol instead of
  duplicating its formula
- Add regression tests for split(f, 0) on a genuine 0-leg tree and
  for multi_associator's early-exit branch on a GenericFusion sector

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix planar issues after MPSKit test rerun

* harden Mooncake scalar pullback

---------

Co-authored-by: Claude Sonnet 5 <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.

3 participants