refactor(pybind)!: guard UniTensor +/- on matching metadata; drop * / (#934)

[yingjerkao]

Decision

Maintainer ruling (yingjerkao, 2026-07-06, amended) resolving #934, #753, and #675: elementwise UniTensor ⊙ UniTensor arithmetic splits by whether the operation is well defined.

• + - += -= are kept, but guarded. Addition/subtraction of two UniTensors is a meaningful vector-space operation when the operands describe the same tensor slot. The Python dunders now require matching metadata and preserve the shared labels; on a mismatch they raise a guidance TypeError instead of silently discarding labels.
• * / *= /= are removed. The elementwise (Hadamard) product/quotient of two UniTensors is basis-dependent and has no tensor-network meaning. Their dunders raise TypeError.

(This amends the earlier ruling, which removed all eight operators from the Python surface.)

What changed

• The four add/sub dunders check that the two operands match on uten_type, rank, labels (in order), rowrank, is_diag, and bonds (unitensor_addsub_metadata_mismatch). On a match they perform the op and restore the shared labels/name — linalg::Add/Sub reset labels to a plain range and clear the name, which is exactly the [BUG] UniTensor exposes raw element-wise array operations that are not meaningful tensor-network operations #934 label-discarding complaint. On a mismatch they raise a TypeError naming the specific difference and suggesting permute()/relabel_() to align.
• The four mul/div dunders raise TypeError with guidance pointing at Contract()/Kron()/ut.get_block().
• Scalar ↔ UniTensor arithmetic (both directions, out-of-place and in-place) is fully retained and dtype-preserving.
• C++ operator+/operator- are untouched — the Krylov solvers (Lanczos_Gnd_Ut.cpp, Lanczos_Exp.cpp) depend on them as vector-space (axpy) operations, and they carry the canonical internal/advanced doxygen note added earlier in this PR.

Semantics of "matching metadata"

Order-sensitive: same uten_type, rank, labels (same order), rowrank, is_diag, and bonds (via Bond::operator==). a["i","j"] + a["j","i"] raises rather than auto-permuting — align the operands first. This keeps the sum/difference unambiguous and label-preserving. (For symmetric BlockUniTensor, the C++ Add_ already enforces matching bonds/is_diag; the Python guard adds the label check and a uniform TypeError.)

BREAKING CHANGE (Python users)

• ut1 * ut2 / ut1 / ut2 (and *= / /=) now raise TypeError.
• ut1 + ut2 / ut1 - ut2 (and += / -=) now raise TypeError when the operands' metadata do not match (previously they silently discarded labels). With matching metadata they work and preserve labels.

The error messages carry the full migration guidance.

Testing

pytests/binding_unitensor_elementwise_test.py (20 tests): matching-metadata +/-/+=/-= work and preserve labels; mismatched labels/bonds raise TypeError; *///*=//= raise; scalar paths retained. The non-example pytest suite (68 passed, 1 skipped) and the DMRG dense / TDVP1 / ED Ising examples (4 passed, exercising the surviving C++ solver arithmetic end-to-end) confirm no regression.

Stacking

Base PR #991 is now merged; this branch is rebased onto master and merges cleanly.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request removes elementwise UniTensor-vs-UniTensor arithmetic operations from the Python surface (raising a TypeError with guidance) as they are not well-defined tensor-network operations, while keeping them in the C++ layer where they are used internally. It also adds comprehensive documentation explaining this decision and unit tests to verify the new behavior. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[ianmccul]

(Unitensor + Unitensor) and (UniTensor - UniTensor) are definitely meaningful and essential operations. As are (UniTensor * Scalar) and (UniTensor / Scalar).

The non-meaningful operations are (UniTensor +- Scalar) and (UniTensor */ UniTensor)

[ianmccul]

From #934:

The following should be removed or disabled:

UniTensor * UniTensor
UniTensor / UniTensor
UniTensor + Scalar
Scalar + UniTensor
UniTensor - Scalar
Scalar - UniTensor
Scalar / UniTensor
UniTensor::Pow
UniTensor::Inv  // if element-wise
UniTensor % ...

For comparison, these operations do make sense on UniTensor, at least conceptually:

• Add_(const UniTensor_base&) and Sub_(const UniTensor_base&), if the tensor spaces/bonds match exactly.
• Mul_(const Scalar&), scalar multiplication.
• Div_(const Scalar&), scalar division.
• contract, if leg ordering, symmetry, and fermionic/braiding semantics are handled correctly.
• Trace, if the traced legs are compatible.
• Conj, Dagger, Norm, normalize.
• permute, provided it handles fermionic signs/braiding correctly.
• astype, clone, to(device), and metadata accessors.

I don't have any good suggestion for resolving #753 and #675. Certainly if both tensors have the same labels and in the same order and the bonds match, then preserving them is a good idea. If they have the same labels but in a different order, it should probably be an error; the alternative is some implicit permutation/braiding which should be a deliberate operation. I have no suggestion of what to do if some labels match but others do not.

The typical operation will be something like y += a * matvec(x), so its the matvec contraction that needs to get the labels correct for that operation. Linear algebra operations "don't care", just want to get operations such as y += a * matvec(x) and inner products etc to work, do not need to know what x and y actually are or anything about labels.

[pcchen]

Code Review

Implements the maintainer ruling to remove elementwise UniTensor (+) UniTensor arithmetic from the Python surface (dunders now raise a guidance TypeError via a [[noreturn]] helper), while deliberately keeping the C++ operators the Krylov solvers depend on. Clean, focused, and unusually well-documented.

What's done well

• Correct mechanism. The UniTensor-RHS overload is registered first and matches ut1+ut2 exactly, so its body runs and throws py::type_error -> propagates as a Python TypeError (pybind does not fall back to __radd__/NotImplemented when a selected overload throws). Scalar/numpy overloads are untouched, so ut + 2.0 etc. still work. ✅
• [[noreturn]] + declared return types (-> UniTensor / -> UniTensor &) compile cleanly — verified by green wheel builds on macOS x2 + Ubuntu x2. std::format is fine (<format> included; already used elsewhere in the file). ✅
• C++ layer correctly preserved — the audit-backed reasoning (Lanczos_Gnd_Ut / Lanczos_Exp use +/- as axpy) is sound, and the doxygen cross-references are thorough and honest (including admitting *// have no in-tree caller). ✅
• Excellent tests + guidance messages — 16 tests (8 removal with ordered-token pins on 934.*Contract/Kron/get_block, 8 scalar-survival), and the error strings give real migration paths. ✅

Findings

1. Incomplete removal — // and //= still perform elementwise division (should-fix; it defeats the stated goal).
__floordiv__(UniTensor, UniTensor) (unitensor_py.cpp:1612) and __ifloordiv__(UniTensor, UniTensor) (:1708) were overlooked — they still call linalg::Div(self, rhs) / self.Div_(rhs). So after this PR:

ut1 / ut2    # TypeError (removed) ✅
ut1 // ut2   # silently returns linalg::Div(ut1, ut2) — the removed op, via a different dunder ❌
ut1 //= ut2  # silently self.Div_(rhs) ❌

This leaves a live bypass for exactly the label-discarding/basis-dependent operation #934 targets, and makes the surface inconsistent (/ gone, // kept). The description enumerates "8 dunders (+ - * / += -= *= /=)" — it should be 10, adding // and //= to raise_unitensor_elementwise_removed (reuse kUniTensorDivRemovedGuidance) plus two tests. Bonus: this also removes a pre-existing oddity where UniTensor // was aliased to true division rather than floor division.

2. CI gap — the test suite hasn't demonstrably run on this commit. The visible checks show wheels/formatting/docs green but no BuildAndTest/pytest job (the branch is behind master; the run looks partial/stale). Compilation is verified, but the 16 new tests' only evidence is the local "71 passed." Recommend a fresh full CI run before merge (as with #989/#991).

3. Breaking change — needs a changelog/release note. Well-flagged in the title (!), body, and error messages, but a user-facing removal like this should land a CHANGELOG entry + a minor/major version signal so downstream ut1 + ut2 breakage is discoverable.

4. Minor (acknowledged): *// C++ operators kept un-deprecated despite no in-tree caller — honestly documented as out-of-scope, flagged for follow-up. Fine.

Verdict

Strong, well-documented refactor with correct mechanics and great tests — but the removal is incomplete: /////= slip through and still perform the very operation being removed. I'd fix that (finding #1) and re-run full CI before merging. Requesting changes on the one real gap.

Posted by Claude Code on behalf of @pcchen

[pcchen]

@yingjerkao Please complete the removal.

[ianmccul]

I am lost for words. A tensor network library that doesn't allow addition of tensors?

[pcchen]

Technical summary: which UniTensor arithmetic is well-defined?

Following @ianmccul's points, here is an operation-by-operation summary to help settle the direction, since the disagreement is about the axis of the removal rather than a detail.

A UniTensor is an element of a labeled tensor-product vector space with (block-sparse) symmetry structure. That fixes which operations are mathematically sound:

Operation	Well-defined?	Why
T ± T (same bonds/labels)	Yes — essential	Tensors form a vector space; adds corresponding blocks; the sum of two symmetric tensors is symmetric. This is y = a·x + b·z — the backbone of Krylov/DMRG/TDVP.
scalar · T, T · scalar, T / scalar	Yes	Scaling every block preserves the zero/symmetry pattern.
T ± scalar, scalar ± T	No	Adds a constant to every element, including entries symmetry forces to zero → destroys block-sparsity/symmetry. Not a canonical TN op.
scalar / T	No	Elementwise reciprocal; basis-dependent, inf/nan.
T * T, T / T (Hadamard)	No	Elementwise product/quotient; basis-dependent, no TN meaning, breaks block structure.

This partition matches #934's own list (remove T*T, T/T, T±scalar, scalar±T, scalar/T, Pow/Inv/%; keep Add_/Sub_(UniTensor), Mul_/Div_(Scalar)).

What this PR does vs. that partition

	Correct action	This PR
T ± T	keep	❌ removes (raises TypeError)
T * T, T / T	remove	✅ removes
T // T, T //= T	remove	❌ still performs it (see my earlier review)
T ± scalar, scalar ± T	remove	❌ keeps
scalar / T	remove	❌ keeps
scalar·T, T·scalar, T/scalar	keep	✅ keeps

The PR is inverted on the additive axis: it removes the meaningful T ± T and retains the ill-defined T ± scalar / scalar / T.

The decisive internal contradiction

This PR keeps the C++ operator+/operator- on UniTensor, explicitly justified in its own doxygen note because the Lanczos/TDVP solvers rely on them as "genuine vector-space arithmetic (axpy)." If T + T is a legitimate vector-space operation in C++, it is equally legitimate from Python — removing it only from the Python surface is inconsistent.

On #753/#675 (the label footgun)

The real problem those issues raise is that T1 + T2 with mismatched labels silently misaligns/discards labels. The remedy is to guard ± (require matching labels; error on same-labels-different-order rather than implicitly permuting), not remove ± — that preserves the essential operation while closing the footgun.

Bottom line

#934's text supports keeping T ± T. Suggest resolving direction before further work here: at minimum keep T ± T (guarded on labels) and move T ± scalar / scalar ± T / scalar / T into the removed set. The T*T, T/T (and the missed //, //=) removals stand either way.

I'll revise my review verdict accordingly: this needs a direction decision between the maintainers, not just the // completion.

Posted by Claude Code on behalf of @pcchen

[pcchen]

Here is what I thought now.

T1 + T2 is allowed only when bond structure and bond labels and bond ordering are exactly the same. For example, if T1 and T2 are of the same shape, but with different label, T1 + T2 will raise an error. If user is sure that T1 + T2 make sense, then user has to explcitly relabel/permute T1 and T2 so that all bond metadata maches.

[yingjerkao]

Yes, we should keep tensor addition and subtraction when the bonds/quantum numbers match.

[codecov]

Codecov Report

❌ Patch coverage is 49.27536% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.94%. Comparing base (b19d68c) to head (340c8e8).
⚠️ Report is 2 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
pybind/unitensor_py.cpp	49.27%	4 Missing and 31 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #997      +/-   ##
==========================================
+ Coverage   31.82%   31.94%   +0.11%     
==========================================
  Files         230      230              
  Lines       33124    33185      +61     
  Branches    13852    13885      +33     
==========================================
+ Hits        10543    10601      +58     
+ Misses      15539    15497      -42     
- Partials     7042     7087      +45     

Flag	Coverage Δ
cpp	31.57% <49.27%> (+0.11%)	⬆️
python	61.84% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
C++ backend	32.61% <ø> (+0.05%)	⬆️
Python bindings	24.64% <49.27%> (+0.62%)	⬆️
Python package	61.84% <ø> (ø)

Files with missing lines	Coverage Δ
include/UniTensor.hpp	50.95% <ø> (-0.14%)	⬇️
pybind/unitensor_py.cpp	25.55% <49.27%> (+1.83%)	⬆️

... and 4 files with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b19d68c...340c8e8. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pcchen]

// and //= (UniTensor ÷ UniTensor) bypass the / removal

While / and /= on two UniTensors now correctly raise TypeError, the floor-division operators are still wired to Div:

.def("__floordiv__",  [](UniTensor &self, const UniTensor &rhs) { return linalg::Div(self, rhs); })
.def("__ifloordiv__", [](UniTensor &self, const UniTensor &rhs) { return self.Div_(rhs); })

So after this PR:

ut1 / ut2     # TypeError — Hadamard quotient removed ✅
ut1 // ut2    # silently returns linalg::Div(ut1, ut2) — the *same* removed op ❌
ut1 //= ut2   # silently self.Div_(rhs) ❌

Two problems, and they compound:

1. It's a bypass of the removal. /////= perform exactly the element-wise Hadamard quotient that ///= were removed for — the operation is still fully reachable, just under a different operator.
2. It's also mislabeled. These are Python's floor-division operators, but they call Div (true element-wise division) — no flooring. So on a UniTensor // just aliases /, which is surprising in its own right.

Suggested fix (consistent with the *// handling): route the UniTensor-RHS __floordiv__/__ifloordiv__ through raise_unitensor_elementwise_removed(" // ", …) / (" //= ", …) using the div-guidance string, and add two tests (ut1 // ut2 and ut1 //= ut2 raise), mirroring the existing __truediv__/__itruediv__ cases.

Posted by Claude Code on behalf of @pcchen

[pcchen]

Ruling needed: scalar ± UniTensor and scalar / UniTensor

The tensor–tensor axis is now settled (keep guarded T ± T, remove T * T / T / T). One set of operations remains where the amended PR and #934's original list disagree, and it needs an explicit maintainer ruling:

Operation	#934 list (@ianmccul)	Current PR (340c8e8b)
UniTensor + Scalar	remove	kept
Scalar + UniTensor	remove	kept
UniTensor - Scalar	remove	kept
Scalar - UniTensor	remove	kept
Scalar / UniTensor	remove	kept

(Note: Scalar * UniTensor, UniTensor * Scalar, UniTensor / Scalar — scalar scaling — are uncontroversial and kept by everyone.)

Technical considerations, for the decision:

• ± Scalar adds a constant to every stored block element. For a block/symmetric UniTensor this does not fill symmetry-forbidden entries (those aren't stored), so it doesn't corrupt block structure — but it's still not a symmetry-covariant operation, and T + c is not the same as T + c·I. So it "works" mechanically but has questionable tensor-network meaning.
• Scalar / UniTensor is an element-wise reciprocal of the stored blocks: basis-dependent, and produces inf/nan wherever a stored element is 0.

Two coherent options:

1. Remove them (follow [BUG] UniTensor exposes raw element-wise array operations that are not meaningful tensor-network operations #934's list) — treat ± Scalar and Scalar / as ill-defined, alongside the * / UniTensor removals; keep only scalar scaling (* and / Scalar).
2. Keep them (amend [BUG] UniTensor exposes raw element-wise array operations that are not meaningful tensor-network operations #934) — accept the "operate on stored blocks" semantics as good enough, and update the [BUG] UniTensor exposes raw element-wise array operations that are not meaningful tensor-network operations #934 record accordingly.

@yingjerkao — the current PR keeps all five; is that a deliberate scope decision or just out of scope for this pass?

Posted by Claude Code on behalf of @pcchen

[ianmccul]

I thought UniTensor + Scalar throws an exception for Block and Fermionic cases?

[yingjerkao]

Two semantic choices in the metadata guard worth a reviewer's eye — both easy to change if you'd prefer different behavior:

1. Label matching is order-sensitive (strict), no auto-permute.
unitensor_addsub_metadata_mismatch requires a.labels() == b.labels() in the same order, so a.relabel(["i","j"]) + a.relabel(["j","i"]) raises TypeError rather than transposing b to align with a before adding. I went strict because it's unambiguous and keeps the guard cheap; the alternative (permute the RHS to the LHS's label order when the label sets match) is more convenient but adds a hidden transpose + a copy. Easy to relax later if you want numpy-broadcast-style convenience.

2. rowrank and is_diag are part of the match.
Two tensors with identical labels/bonds but different rowrank (row/col leg split) or different is_diag will raise. Rationale: they describe genuinely different tensor slots (rowrank changes contraction semantics; a diagonal vs dense block differ in storage/meaning). This is stricter than the C++ Add_, which for DenseUniTensor keys off bonds/shape and would happily add across a rowrank difference. If you'd rather have +/- ignore rowrank (treat it as pure bookkeeping for untagged Dense tensors), I can drop it from the check.

For reference, the full match criteria are: same uten_type, rank, labels (in order), rowrank, is_diag, and bonds (via Bond::operator==). On a match the op runs and the shared labels/name are restored (since linalg::Add/Sub reset them); on a mismatch the TypeError names the specific difference.

[yingjerkao]

We should allow UniTensor/(Rank-0 UniTensor/Tensor)

[pcchen]

Resolution of the open review items — this PR is now approvable as-is

Maintainer direction (@pcchen) resolving the two items left open in review:

1. The scalar-operand ruling (my "Ruling needed" table above — UniTensor ± Scalar, Scalar ± UniTensor, Scalar / UniTensor): resolved by the rank-0 path rather than in this PR.

• fix(Tensor & UniTensor): support rank-zero tensors and zero extents #1026 (@ianmccul) implements dense rank-0 tensors, resolving the Design question: should scalars be rank-0 (shape-{}) tensors? #1016 design question.
• After fix(Tensor & UniTensor): support rank-zero tensors and zero extents #1026 merges, a follow-up issue will be filed to implement UniTensor / (rank-0 UniTensor/Tensor) — the principled, tensor-typed scalar division (dtype/symmetry-aware, the normalize_-style broadcast) that supersedes the ad-hoc Python-scalar-division semantics debated here. The Python-scalar operands this PR keeps remain as-is until that lands; no change required in this PR.

2. The /////= bypass finding: superseded by direction — integer dtypes are slated for removal from UniTensor, so floor division has no future operand set and // will be unbound entirely (as #1015 already implements per #941) rather than given raise-with-guidance handling here. No change required in this PR.

With both items resolved by direction, what this PR delivers stands on its own and matches the agreed rulings: guarded T ± T (matching uten_type/rank/labels/rowrank/is_diag/bonds, label-and-name-preserving on match), T * T / T / T removed with guidance errors, scalar arithmetic untouched. Review verdict updates to approve.

Posted by Claude Code on behalf of @pcchen

[pcchen]

Correction to point 1 above (maintainer ruling, @pcchen)

UniTensor ± Scalar and Scalar ± UniTensor will NOT be supported — the ruling adopts #934's original list for the additive scalar operands (adding a constant to every stored element is not a tensor-network operation), not the keep-as-is reading my previous comment implied.

Updated resolution map for the scalar operands:

Operation	Ruling
T ± scalar, scalar ± T	Removed (this ruling; no replacement needed — the operation is ill-defined on a TN object)
T / scalar (and T * scalar, scalar * T)	Kept — scalar scaling is well-defined; the principled tensor-typed form (T / rank-0) arrives via the #1026 follow-up issue
scalar / T	Remains in the remove set per #934; the #1026 follow-up issue will settle whether any rank-0 reciprocal form exists

The ± scalar removal is follow-up implementation work (it can ride the same follow-up as the rank-0 division, or land as its own small PR mirroring this PR's guidance-error pattern) — per maintainer direction it is not a blocker for merging this PR, which already delivers the agreed tensor-tensor semantics. Verdict unchanged: approve.

Posted by Claude Code on behalf of @pcchen

[pcchen]

Approving. The amended direction is correct and verified: T ± T is kept and guarded on matching metadata (uten_type, rank, labels in order, rowrank, is_diag, per-leg bonds), performing with the shared labels and name restored on match — which directly fixes the #753/#675 label-discarding complaint — and raising a specific, actionable TypeError on mismatch; T * T / T / T (Hadamard) are removed with guidance errors; scalar arithmetic and the C++ solver-facing operator+/- are untouched. Tests pin all of it (matching-metadata perform ×4, mismatch raises, removal raises, scalar survival ×8) and CI is fully green (16 pass).

The remaining semantic questions are resolved by documented maintainer rulings rather than in this PR: UniTensor ± Scalar / Scalar ± UniTensor are ruled not supported (removal is follow-up work mirroring this PR's pattern); tensor-typed scalar division arrives via the #1026 rank-0 follow-up; and /////= will be unbound entirely per the integer-removal direction (#1015 implements this) — none block this merge.

Posted by Claude Code on behalf of @pcchen