Skip to content

Enforce strict Einstein index rules in @tensor - #289

Merged
lkdvos merged 2 commits into
masterfrom
ld-indexing
Jul 31, 2026
Merged

Enforce strict Einstein index rules in @tensor#289
lkdvos merged 2 commits into
masterfrom
ld-indexing

Conversation

@lkdvos

@lkdvos lkdvos commented Jul 30, 2026

Copy link
Copy Markdown
Member

Fixes #288.

@tensor silently accepted expressions where an index label appears more than twice, e.g.

@tensor T[-1 -2 -3; -4 -5] := A[-2 1] * B[1 -3; 1 -5] * C[-1 1; -4]

The only arity check lived in insertcontractiontrees! and counted getindices per factor.
Since getindices returns open indices, a label traced away inside one factor became
invisible and could be reused by the other factors. The check also never ran for products of
fewer than three factors, nor for a single tensor.

This adds verifyindices, a single pass over each statement that enforces the documented rule:
within one additive term every label appears once (open) or exactly twice (contracted), counting
all occurrences including traces. Parentheses group factors but do not open a new label scope;
different terms of a sum, different statements and explicit tensorscalar arguments do.
Violations throw an ArgumentError at macro expansion time. The pass runs as a preprocessor
right after normalizeindices (so primed labels compare equal) and replaces the partial check in
contractiontrees.jl; it also covers labels appearing more than twice in a single tensor and
terms of a sum with differing open indices, which previously surfaced as confusing runtime
IndexErrors.

Note this rejects code that used to run: three @test_throws IndexError cases in
test/tensor.jl / test/cutensor.jl now fail at expansion instead and were updated. Scanning
the @tensor calls in TensorKit, MPSKit, MPSKitModels, PEPSKit and TensorKitManifolds turned up
no violations on current main. It does flag PEPSKit v0.3.0's right_projector, where a trace
label was reused across factors -- a bug that had to be found and fixed by hand in
QuantumKitHub/PEPSKit.jl@9b43dad, and which this check would have caught at macro expansion.

🤖 Generated with Claude Code

@lkdvos
lkdvos requested review from Jutho and VictorVanthilt July 30, 2026 17:54
@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.61111% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/indexnotation/verifiers.jl 98.61% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/indexnotation/contractiontrees.jl 100.00% <ø> (ø)
src/indexnotation/parser.jl 93.90% <ø> (+1.21%) ⬆️
src/indexnotation/verifiers.jl 95.23% <98.61%> (+6.69%) ⬆️

... and 4 files with indirect coverage changes

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

Cover the paths of `verifyindices` that were not yet exercised:

- `/` and `\`: the divisor is verified as a separate scope, while the
  closed labels of the tensor operand do escape to the enclosing term.
- `conj`, adjoint and unary minus, which pass the labels through.
- `@notensor`, which is skipped entirely. This also adds the first
  runtime test for `@notensor`, covering the corresponding branches in
  `processcontractions`, `replaceindices` and `tensorify`.
- the argument of an explicit `tensorscalar`, which is a separate scope
  but is still verified.

Two branches turned out to be unreachable and are removed instead: an
explicit `tensorscalar(...)` call is not an `istensorexpr`, and a sum
node cannot carry scalar terms since `istensorexpr` requires all of its
terms to be tensor expressions. Both cases are already handled through
the scalar factor fallback of `_indexscope_product`.

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

Copy link
Copy Markdown
Member

I'm not familiar at all with programming macros and working with expressions so perhaps @Jutho best review this.

@Jutho

Jutho commented Jul 31, 2026

Copy link
Copy Markdown
Member

Looks good to me. Was this logic completely Claude generated? Not bad.

@lkdvos

lkdvos commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Yeah, it mostly is, although I did steer it a little and had to clean up some things in the end.

@lkdvos
lkdvos merged commit 3daa8fe into master Jul 31, 2026
13 checks passed
@lkdvos
lkdvos deleted the ld-indexing branch July 31, 2026 13:32
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.

Syntactically incorrect @tensor call runs.

3 participants