fixed reversed qnums infixed reversed qnums in linalg functions if combined incoming bond BdLeft is outgoing instead of incoming (#761 #763) linalg functions

[manuschneider]

This happens if the combined incoming bond BdLeft is outgoing instead of incoming.
Fixes #761 and #763

[codecov]

Codecov Report

❌ Patch coverage is 77.77778% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 35.58%. Comparing base (397ecbc) to head (515a2c2).
⚠️ Report is 57 commits behind head on master.

Files with missing lines	Patch %	Lines
src/linalg/Gesvd.cpp	50.00%	3 Missing ⚠️
src/linalg/Svd.cpp	50.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #764      +/-   ##
==========================================
+ Coverage   35.44%   35.58%   +0.13%     
==========================================
  Files         215      214       -1     
  Lines       33071    33034      -37     
  Branches    13170    13159      -11     
==========================================
+ Hits        11723    11756      +33     
+ Misses      19424    19352      -72     
- Partials     1924     1926       +2     

☔ 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.

[pcchen]

PR Review: #764

Fix assessment: Correct — using calc_reverse_qnums() when BdLeft.type() == BD_OUT is the right approach and addresses #761 / #763.

---

Important Issues

1. calc_reverse_qnums() called inside the loop — O(Nblocks) recomputation

Files: All 9 patched locations (Eig.cpp:191, Eigh.cpp:184, ExpH.cpp:158, ExpM.cpp:144, Gesvd.cpp:240/466, Qr.cpp:221, Svd.cpp:249/474)

In the else branch, calc_reverse_qnums() is called on every loop iteration, recomputing the full reversed vector each time. It should be hoisted out:

const auto left_qnums = (BdLeft.type() == bondType::BD_IN)
                         ? BdLeft.qnums()
                         : BdLeft.calc_reverse_qnums();
for (cytnx_uint64 b = 0; b < Tin.Nblocks(); b++) {
  mgrp[left_qnums[new_itoi[b][0]]].push_back(b);
}

Note: qnums() likely returns a const ref while calc_reverse_qnums() returns by value, so use auto (not const auto&) to avoid a dangling reference.

2. Missing regression tests for 6 of 7 patched functions

Affects: Eig, Eigh, ExpH, ExpM, Gesvd, Qr

Only Svd got a regression test. The same BD_OUT BdLeft bug class exists in all other patched functions and is entirely untested. This is especially risky for:

• Gesvd and Qr — used heavily in DMRG/TDVP; wrong qnums silently corrupt the subspace and propagate through the simulation
• ExpH and ExpM — no Block UniTensor tests exist at all in those test files; this is a good opportunity to add them

The SVD test pattern is a good template — each new test should verify both qnums() and getDegeneracies() on the auxiliary bond (not just ReComposeCheck).

---

Suggestions

3. Bare else — no guard for unexpected bond types

The else branch silently handles any non-BD_IN type. An explicit guard would be more defensive:

} else if (BdLeft.type() == bondType::BD_OUT) {
  // ...
} else {
  cytnx_error_msg(true, "[ERROR] unexpected BdLeft bond type in block categorization", "\n");
}

4. SVD test uses a degenerate single-sector tensor

File: tests/linalg_test/Svd_test.cpp

The regression test has only one qnum sector ({4} with degeneracy 1). A multi-sector test (e.g. Bond(BD_OUT, {{0},{1},{2}}, {2,3,2}, {Symmetry::U1()})) would give higher confidence that calc_reverse_qnums() indexing is correct across all sectors.

---

Strengths

• The fix logic is correct and well-understood.
• The SVD regression test explicitly checks both qnum values and degeneracies on the auxiliary bond — stronger than a pure round-trip recompose check.
• The fix is applied consistently across all 9 locations.

---

Recommended Action

1. Before merge: Hoist calc_reverse_qnums() out of the loop (applies to all 9 sites).
2. Before merge: Add regression tests for at minimum Gesvd and Qr (highest real-world impact).
3. Can follow up: Tests for Eig, Eigh, ExpH, ExpM in a follow-on PR.
4. Optional: Add defensive else if guard.

---

Posted by Claude Code on behalf of @pcchen

[manuschneider]

1. Resolved
2. & 4. Could the tests be improved with some AI help? I only implemented the case in linalg.Svd produces wrong (negative) auxiliary bond qnums #763 as a test so far.
3. We currently only support types BD_IN or BD_OUT for a BlockUniTensor, so need for a check here.