[BREAKING] Drop per-edge index wrappers + non-OpSum ttn ctors; rename scale -> scale_tensors#355
Merged
Merged
Conversation
…appers Followup to #354. Removes the deprecated/experimental `AbstractITensorNetwork`- and `AbstractIndsNetwork`-side per-edge index wrappers in favor of the kept `linkinds(tn, edge)` accessor and explicit `intersect`/`setdiff` calls on raw `inds(...)` at call sites. ## Deletions - `commoninds(tn::AbstractITensorNetwork, edge)` — was a thin alias for `commoninds(tn[src(e)], tn[dst(e)])`. The kept `linkinds(tn, edge)` accessor returns the same thing. The two were aliases anyway. - `uniqueinds(tn::AbstractITensorNetwork, edge::AbstractEdge)` and the `Pair` overload — was `uniqueinds(tn[src(e)], tn[dst(e)])`. - `hascommoninds(tn::AbstractITensorNetwork, edge::AbstractEdge|Pair)` — was `hascommoninds(tn[src(e)], tn[dst(e)])`. - `uniqueinds(is::AbstractIndsNetwork, edge::AbstractEdge|Pair)` — walked the IndsNetwork to collect site inds at `src(edge)` plus link inds on all other edges incident to `src(edge)`. The single internal caller (in `ttn(::ITensor, ::IndsNetwork)`) inlines a semantically-equivalent rewrite using the already-built `tn`. ## Reimplementation `linkinds(tn::AbstractITensorNetwork, edge)` is kept (parallel to the also-kept `siteinds(tn, vertex)`) and reimplemented natively as `intersect(inds(tn[src(e)]), inds(tn[dst(e)]))` instead of delegating to the deleted `commoninds(tn, edge)` wrapper. ## Call site rewrites - `weights(::AbstractITensorNetwork)` (`abstractitensornetwork.jl:34`) — `commoninds(graph, e)` → `linkinds(graph, e)`. The misleadingly-named `graph` argument is actually an `AbstractITensorNetwork`; this site was relying on the deleted wrapper. - `IndsNetwork(tn)` and 1-arg `linkinds(tn)` (`abstractitensornetwork.jl` lines 249, 267) — `commoninds(tn, e)` → `linkinds(tn, e)`. - `fix_edges!`, `union(tn1, tn2)` (lines 138, 183) — `hascommoninds(tn, e)` → `!isempty(linkinds(tn, e))`. - `insert_linkinds` (line 848) — `!hascommoninds(tn, e)` → `isempty(linkinds(tn, e))`. - `LinearAlgebra.svd`/`qr`/`factorize`, `gauge_edge`, `_truncate_edge` (5 sites) — `uniqueinds(tn, edge)` → `setdiff(inds(tn[src(edge)]), inds(tn[dst(edge)]))`. - `ttn(::ITensor, ::IndsNetwork)` loop in `treetensornetwork.jl:233` — `uniqueinds(is, e)` → `setdiff(inds(a), inds(tn[dst(e)]))`. Equivalent given the post-order-DFS invariant: `tn[dst(e)]` retains its original inds (parent vertices aren't processed as `src` until after their subtree), so the only inds shared with the running accumulator `a` are the link inds on edge `e`. - 2 sites in `test/test_itensornetwork.jl` "Index access" testset. ## Doc sync Removes the corresponding entries from `docs/src/deprecated_methods.md` (per-edge `commoninds` / `uniqueinds` / `linkinds` block) and `docs/src/experimental_methods.md` (IndsNetwork-side `uniqueinds`). Drops dead imports (`hascommoninds`, `unioninds`, `uniqueinds` from `abstractitensornetwork.jl`; `ITensors`, `IndexSet`, `uniqueinds` from `abstractindsnetwork.jl`; `hascommoninds`, `uniqueinds` from `test/test_itensornetwork.jl`). ## Version bump `0.20.0 → 0.21.0` (breaking — public methods removed). The pin in `test/`, `examples/`, and `docs/` `Project.toml` is bumped from `"0.20"` to `"0.21"` to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit dropped `ITensors` (the module name itself) from this file's import alongside the now-unused `IndexSet` and `uniqueinds`. That broke a transitive dependency: many other files in `src/` use qualified `ITensors.foo(...)` syntax (e.g. `ITensors.ITensor(o, s)` in `src/opsum.jl:7`) but only specific names from ITensors, not the module itself. They were relying on `abstractindsnetwork.jl` (loaded early in the include chain) to bring `ITensors` into the module's namespace. Local tests passed because Revise's in-memory module had the symbol already, but cold precompilation in CI failed at `src/opsum.jl:7` with `UndefVarError: ITensors not defined in ITensorNetworks`. Minimal fix: re-add `ITensors` (module name) to this file's import. The proper long-term fix is for each file that uses `ITensors.foo` qualification to import the module itself, but that's a separate refactor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #355 +/- ##
==========================================
+ Coverage 74.40% 74.60% +0.20%
==========================================
Files 64 64
Lines 3133 3103 -30
==========================================
- Hits 2331 2315 -16
+ Misses 802 788 -14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ope was lost Continuing the precompile fix from the previous commit. Several files across `src/` call bare `hascommoninds(t1::ITensor, t2::ITensor)` or `uniqueinds(t1::ITensor, t2::ITensor)` (both ITensors functions on ITensor pairs — these are legitimate, not the deleted network+edge wrappers) but don't import those names themselves. They were relying on `abstractitensornetwork.jl`'s top-level `using ITensors: ..., hascommoninds, unioninds, uniqueinds, ...` to bring those names into the `ITensorNetworks` module namespace via transitive scope. The wrapper deletion in this PR removed those imports from `abstractitensornetwork.jl` (since the wrappers it defined were the only local users), which in turn breaks the transitive scope downstream files were depending on. Cold precompile fails; Revise's incremental updates mask the problem locally. Files updated (each gets only the names it actually uses): - `src/apply.jl` — adds `hascommoninds` to the existing `using ITensors: ...` line. - `src/solvers/subspace/densitymatrix.jl` — adds `using ITensors: hascommoninds, uniqueinds`. - `src/solvers/applyexp.jl` — adds `using ITensors: uniqueinds`. - `src/treetensornetworks/projttns/projttn.jl` — adds `hascommoninds` to the existing `using ITensors: ITensor` line. - `src/treetensornetworks/projttns/projouterprodttn.jl` — same. `src/solvers/subspace/ortho_subspace.jl` already does bare `using ITensors`, which brings everything into scope, so it's unaffected. These imports made the transitive dependency that already existed explicit. No semantic changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…scale_tensors`/`scale_tensors!`
Folding two more breaking changes into this PR to consolidate the
0.20 -> 0.21 bump rather than churning multiple breaking releases.
## Drop non-`OpSum` `ttn` constructors
Removes 6 thin wrappers in `src/treetensornetworks/opsum_to_ttn/opsum_to_ttn.jl`
that all boil down to `ttn(OpSum{...}() + o, s; kwargs...)`:
```julia
ttn(o::Op, s::IndsNetwork; kwargs...)
ttn(o::Scaled{C, Op}, s::IndsNetwork; kwargs...)
ttn(o::Sum{Op}, s::IndsNetwork; kwargs...)
ttn(o::Prod{Op}, s::IndsNetwork; kwargs...)
ttn(o::Scaled{C, Prod{Op}}, s::IndsNetwork; kwargs...)
ttn(o::Sum{Scaled{C, Op}}, s::IndsNetwork; kwargs...)
```
Zero callers in `src/` or `test/` (only the function definitions
themselves matched). Users with `Op`-shaped operators can wrap into an
`OpSum` themselves and call `ttn(opsum, sites)`. The `Op`-adjacent forms
can come back later through Lukas's symbolic operator system if needed.
Removed the corresponding entry from `docs/src/deprecated_methods.md`.
## Rename `scale` / `scale!` -> `scale_tensors` / `scale_tensors!`
The interface-doc comment for these has been flagging them as too
generically named — they collide with broader scaling semantics, and the
intent is "scale individual vertex tensors by per-vertex weights, leaving
the graph alone." Rename clarifies that.
Mechanical: 4 internal callers (`src/normalize.jl` x2,
`src/caches/abstractbeliefpropagationcache.jl` x2), plus the definitions
in `src/abstractitensornetwork.jl` and the entry in
`docs/src/interface_methods.md`. No tests reference the old names.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These are imported on lines 4-5 of `test/test_belief_propagation.jl` but never actually called in the test body. The functions still exist in `src/` (used by `add(tn1, tn2)` to collapse multi-edges) — this just removes the unused test imports. The longer-term plan to drop `combine_linkinds`/`linkinds_combiners` themselves is now noted in the `insert_linkinds` redesign followup (item #11 in `tn_stack_consolidation/Overview.md`): a proper `edge_factorization`/`add_edge!` interface subsumes "collapse a multi-edge into a single canonical bond" as a special case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mtfishman
changed the title
mtfishman
added a commit
that referenced
this pull request
May 9, 2026
…neric ctors (#356) ## Summary Continues the post-#355 cleanup. Drops the rest of the higher-level `ITensorNetwork` and `ttn` / `mps` constructor surface that wrapped the old `IndsNetwork`-based code paths, plus the `ttn(::OpSum, ::IndsNetwork)` and `ttn(::ITensor, ::IndsNetwork)` dispatches (canonical name is now `TreeTensorNetwork`; `mpo` continues to dispatch through to it). Surviving construction surface: `ITensorNetwork(tensors)`, `ITensorNetwork(tensors, graph)`, and `TreeTensorNetwork(::OpSum / ::ITensor / ::ITensorNetwork, ...)`. Adds `factorize_edge!` and `Graphs.add_edge!` on `AbstractITensorNetwork`. Internal callers that built `ITensorNetwork(::IndsNetwork)` placeholders now build `ITensorNetwork(tensors, graph)` directly. Folds in two bug fixes (missing `trivial_space` import; `_siteinds` returning `Tuple` instead of `Vector{Index}`). `test/utils.jl` substantially simplified (~261 → ~75 effective lines): surviving helpers are `random_tensornetwork`, `productstate`, and `ModelHamiltonians`; `Distributions` dep dropped. Docs updated to the canonical construction. `Project.toml` bumped 0.21 → 0.22.0-DEV (breaking; accumulator pattern — a strip-suffix PR will release 0.22.0 once v0.22 cleanups are batched). --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Phase 3 ITensorNetworks.jl cleanup, consolidating multiple breaking changes into a single 0.20.0 -> 0.21.0 bump rather than churning multiple breaking releases.
1. Drop per-edge
commoninds/uniqueinds/hascommonindswrappersRemoves the deprecated/experimental
AbstractITensorNetwork- andAbstractIndsNetwork-side per-edge index wrappers in favor of the keptlinkinds(tn, edge)accessor and explicitintersect/setdiffcalls on rawinds(...)at call sites.Deletions:
commoninds(tn::AbstractITensorNetwork, edge)— was a thin alias forcommoninds(tn[src(e)], tn[dst(e)]). The keptlinkinds(tn, edge)returns the same thing.uniqueinds(tn::AbstractITensorNetwork, edge::AbstractEdge|Pair).hascommoninds(tn::AbstractITensorNetwork, edge::AbstractEdge|Pair).uniqueinds(is::AbstractIndsNetwork, edge::AbstractEdge|Pair)— walked the IndsNetwork to collect site inds atsrc(edge)plus link inds on all other edges incident tosrc(edge). The single internal caller (inttn(::ITensor, ::IndsNetwork)) inlines a semantically-equivalent rewrite using the already-builttn.Reimplementation:
linkinds(tn::AbstractITensorNetwork, edge)is kept (parallel to the also-keptsiteinds(tn, vertex)) and reimplemented natively asintersect(inds(tn[src(e)]), inds(tn[dst(e)]))instead of delegating to the deletedcommoninds(tn, edge)wrapper.Call site rewrites:
Graphs.weights(::AbstractITensorNetwork)—commoninds(graph, e)->linkinds(graph, e).IndsNetwork(tn)and 1-arglinkinds(tn)—commoninds(tn, e)->linkinds(tn, e).fix_edges!,union(tn1, tn2)—hascommoninds(tn, e)->!isempty(linkinds(tn, e)).insert_linkinds—!hascommoninds(tn, e)->isempty(linkinds(tn, e)).LinearAlgebra.svd/qr/factorize,gauge_edge,_truncate_edge(5 sites) —uniqueinds(tn, edge)->setdiff(inds(tn[src(edge)]), inds(tn[dst(edge)])).ttn(::ITensor, ::IndsNetwork)loop —uniqueinds(is, e)->setdiff(inds(a), inds(tn[dst(e)])). Equivalent given the post-order-DFS invariant:tn[dst(e)]retains its original inds (parent vertices aren't processed assrcuntil after their subtree), so the only inds shared with the running accumulatoraare the link inds on edgee.test/test_itensornetwork.jl"Index access" testset.2. Drop non-
OpSumttnconstructorsRemoves 6 thin wrappers in
src/treetensornetworks/opsum_to_ttn/opsum_to_ttn.jlthat all boil down tottn(OpSum{...}() + o, s; kwargs...):Zero callers in
src/ortest/. Users withOp-shaped operators can wrap into anOpSumthemselves and callttn(opsum, sites). TheOp-adjacent forms can come back later through Lukas's symbolic operator system if needed.3. Rename
scale/scale!->scale_tensors/scale_tensors!The interface-doc comment for these has been flagging them as too generically named — they collide with broader scaling semantics, and the intent is "scale individual vertex tensors by per-vertex weights, leaving the graph alone." The rename clarifies that.
Mechanical: 4 internal callers (
src/normalize.jlx2,src/caches/abstractbeliefpropagationcache.jlx2), plus the definitions insrc/abstractitensornetwork.jland the entry indocs/src/interface_methods.md. No tests reference the old names.4. Doc + import housekeeping
docs/src/deprecated_methods.mdanddocs/src/experimental_methods.md.combine_linkinds/linkinds_combinersimports fromtest/test_belief_propagation.jl(imported but never used).commoninds/uniqueinds/hascommoninds-related imports fromsrc/abstractitensornetwork.jlandsrc/abstractindsnetwork.jl.using ITensors: hascommoninds, uniqueindsimports to several files (apply.jl,solvers/subspace/densitymatrix.jl,solvers/applyexp.jl,treetensornetworks/projttns/projttn.jl,projouterprodttn.jl) that were inheriting these names via transitive scope fromabstractitensornetwork.jl's top-level imports. The transitive dependency is now explicit.Version bump
0.20.0 -> 0.21.0(breaking — public methods removed/renamed).Release notes:
Breaking. Removed public methods:
ITensors.commoninds(tn::AbstractITensorNetwork, edge),ITensors.uniqueinds(tn::AbstractITensorNetwork, edge::AbstractEdge|Pair),ITensors.hascommoninds(tn::AbstractITensorNetwork, edge::AbstractEdge|Pair),ITensors.uniqueinds(is::AbstractIndsNetwork, edge::AbstractEdge|Pair), and 6 non-OpSumttnconstructors (ttn(::Op, ...),ttn(::Scaled{C, Op}, ...),ttn(::Sum{Op}, ...),ttn(::Prod{Op}, ...),ttn(::Scaled{C, Prod{Op}}, ...),ttn(::Sum{Scaled{C, Op}}, ...)). Renamedscale/scale!toscale_tensors/scale_tensors!. Thelinkinds(tn::AbstractITensorNetwork, edge)accessor is preserved with a re-implemented body usingintersect. For unique-side and shared-side queries, use explicitsetdiff(inds(tn[src(e)]), inds(tn[dst(e)]))andintersect(inds(tn[src(e)]), inds(tn[dst(e)]))(orlinkinds(tn, e)for the shared case). ForOp/Scaled/Sum/Prodoperator inputs tottn, wrap into anOpSumfirst.