Transpose exchanges roles of left and right indices

[manuschneider]

This fixes #367.

New behavior:

1. The role of left and right indices is exchanged for bosonic UniTensors, while for BlockFermionicUniTensor the order of the indices is inverted.
2. The rowrank is adapted accodingly
3. Incoming bonds become outgoing ones and vice versa (redirect)

Unit tests are adapted accordingly. Some corrections in the unit tests were also done where I saw them (replacing size_t by cytnx_int64 etc.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 35.56%. Comparing base (a22dd1e) to head (4f7612f).
⚠️ Report is 16 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #725      +/-   ##
==========================================
+ Coverage   35.47%   35.56%   +0.08%     
==========================================
  Files         214      214              
  Lines       33007    33031      +24     
  Branches    13130    13140      +10     
==========================================
+ Hits        11709    11747      +38     
+ Misses      19376    19361      -15     
- Partials     1922     1923       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ianmccul]

@codex review

[manuschneider]

@codex review

Did this work?

[chatgpt-codex-connector]

To use Codex here, create a Codex account and connect to github.

[pcchen]

PR #725 Review — Transpose exchanges roles of left and right indices

Posted by Claude Code on behalf of @pcchen

Critical Issues (2)

1. GPU Trace test: 0-indexed loop but {j-1, k-1} access — out-of-bounds when j=k=0
tests/gpu/BlockUniTensor_test.cpp:18–26

The loop variables were updated from 1-indexed to 0-indexed, but the loop body still uses {j-1, k-1}:

// NEW (buggy):
for (cytnx_int64 j = 0; j < 11; j++)
  for (cytnx_int64 k = 0; k < 3; k++)
    if (BUtrT4.at({j - 1, k - 1}).exists()) {   // j-1 = -1 when j=0
      EXPECT_DOUBLE_EQ(tmp.at({j - 1, k - 1}).real(), ...);

When j=0, k=0 this accesses {-1, -1} — out of bounds. The corresponding CPU test (tests/BlockUniTensor_test.cpp:903) correctly uses {j, k} directly. The GPU test was only partially updated.

2. Add, Sub, Mul, Div, Conj, elem_exist CPU tests: second dimension bound silently reduced from 11 to 5
tests/BlockUniTensor_test.cpp:594–760, 800–840, 1051–1070

All six tests changed their second-dimension loop from j <= 11 (→ accesses 0..10) to j < 5 (→ accesses 0..4). BUT4 has bond dimension 11 in that axis (B2p = Bond(BD_OUT, {Qs(-1), Qs(0), Qs(1)}, {4, 3, 4}) = dim 11). The GPU Conj test correctly kept j < 11. The CPU tests now silently skip checking elements at indices 5..10.

---

Important Issues (3)

3. Missing _is_braket_form update in DenseUniTensor::Transpose_() tagged path
src/DenseUniTensor.cpp

The old tagged path explicitly called:

this->_is_braket_form = this->_update_braket();

The new code removes this call and relies on permute_() to maintain the field. If permute_() does not update _is_braket_form, the field will be stale after tagged transpose. No test currently asserts on is_braket_form() after a tagged Transpose_().

4. No test for BlockFermionicUniTensor::Transpose_() rowrank update
src/BlockFermionicUniTensor.cpp:1350–1362

The new line this->_rowrank = idxnum + 1 - this->_rowrank is the main addition to the fermionic path, but no test verifies it. The BlockUniTensor and DenseUniTensor paths both have Transpose test coverage; fermionic tensors don't.

5. A.Dagger().permute_(A.labels()) pattern repeated 6× in examples with no explanation
example/DMRG/dmrg_two_sites_dense.py, example/DMRG/dmrg_two_sites_U1.py, example/TDVP/tdvp1_dense.py

With the new semantics, A.Dagger() permutes indices (not just flips bonds), so callers who want "conjugate with same label order" must manually permute back. This non-obvious two-step pattern now appears 6 times. Either add a comment explaining why in at least one place, or consider a convenience method.

---

Minor Issues (2)

6. Transpose_tagged test: bond redirect loop has no effect
tests/DenseUniTensor_test.cpp:1189–1194

auto Spcd_p = Spcd_t.permute(Spcd.labels());
for (auto bond : bonds_p) { bond.redirect_(); }  // modifies local copies only — no effect on tensor
EXPECT_TRUE(AreEqUniTensor(Spcd_p, Spcd));

The loop modifies auto bond (local copies), not Spcd_p's bonds. The second EXPECT_TRUE does not test what it appears to test.

7. Docstring typo
include/UniTensor.hpp:327: "Incoming legs become outgoing onces" → "once" (or rephrase).

---

Strengths

• The core permutation logic in BlockUniTensor::Transpose_() and DenseUniTensor::Transpose_() (tagged path) is correct: bonds are all redirected, rowrank is updated, and the index permutation [oldrowrank..rank-1, 0..oldrowrank-1] correctly swaps the two groups.
• BlockFermionicUniTensor: size_t → cytnx_int64 fix prevents signed/unsigned comparison warnings.
• Removing debug std::cout from contract tests is a good cleanup.
• set_name() additions to test fixtures make failure messages more informative.
• The Dagger test update correctly validates the new index permutation behavior with element-wise cross-checking at {k,l,i,j}.