RegularNetwork.Launch works for non-dense UniTensors with cutensor

[manuschneider]

this should fix #592

[codecov]

Codecov Report

❌ Patch coverage is 86.84211% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 35.34%. Comparing base (4080739) to head (d2c2667).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
src/RegularNetwork.cpp	86.84%	0 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #750      +/-   ##
==========================================
+ Coverage   35.25%   35.34%   +0.08%     
==========================================
  Files         215      215              
  Lines       33095    33009      -86     
  Branches    13196    13135      -61     
==========================================
- Hits        11669    11668       -1     
+ Misses      19512    19428      -84     
+ Partials     1914     1913       -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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0fa8d45db

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Use cuquantum branch for all CUDA device IDs

The new guard tn_device == Device.cuda only matches GPU 0, because Device.cuda is defined as 0 while this codebase supports Device.cuda+<gpuid> for other GPUs. As a result, dense tensors on GPU 1+ no longer enter the cuquantum path and instead fall back to the generic contraction tree, which also ignores the cuquantum einsum_path produced by setOrder(optimal=true). This is a regression for multi-GPU users and can cause major performance/memory blowups on larger networks.

Useful? React with 👍 / 👎.

[manuschneider]

fixed

[pcchen]

Code Review: PR #750 "RegularNetwork.Launch works for non-dense UniTensors with cuTensor"

Critical Issues

1. The PR title/goal is NOT achieved — non-dense UniTensors still fail at runtime

Inside the #ifdef UNI_CUQUANTUM block (RegularNetwork.cpp:1042–1046), there is still a hard error gate:

if (this->tensors[0].uten_type() != UTenType.Dense) {
    cytnx_error_msg(true, "[ERROR][Launch]...",
                    "Sparse or Block type UniTensor network contraction is not support.\n");
    return UniTensor();  // also unreachable dead code
}

Non-dense UniTensors on any GPU still hit this and throw. The device routing fix is orthogonally correct (multi-GPU improvement), but the PR's stated goal of resolving issue #592 is not accomplished.

2. The cuTensorNet layer fundamentally only supports dense tensors

In src/utils/cutensornet.cu, both setInputMem/setOutputMem call .get_block_() on UniTensors, which returns only the first block for Block/BlockFermionic types. Even if the error guard at line 1042 were removed, passing Block-type UniTensors would silently produce wrong results or crash — the cuQuantum integration layer needs substantial rework to support block-structured tensors.

---

Important Issues

3. Device comparison uses magic literal -1 instead of Device.cpu

Despite the PR description claiming #include "Device.hpp" was added, the comparisons still use raw -1 literals (if (tn_device == -1)) rather than the Device.cpu named constant used throughout the rest of the project.

4. Typo fixes listed in the PR description were not applied

• "psudotensors" → "pseudotensors" still present at RegularNetwork.cpp:949
• "accroding" → "according" still present at lines 1031 and 1139

5. setOrder and Launch have duplicate non-dense guards with no shared strategy

setOrder (line 886) and Launch (line 1042) both independently reject non-dense types with separate error messages. If block-tensor support is eventually added, both need updating with no single place to change. The return UniTensor() after cytnx_error_msg(true, ...) in Launch is also unreachable dead code.

---

Positive Aspects

• Multi-GPU routing fix is correct: The tn_device != -1 check now properly routes GPU 1, 2, etc. to the cuQuantum path, which was broken before (old tn_device == Device.cuda was == 0, GPU 0 only)
• No logic regressions in the cuQuantum conditional compilation restructuring
• CPU fallback path in the #else block is correctly preserved

---

Summary

Area	Verdict
Multi-GPU device routing fix	Correct improvement
Non-dense UniTensors with cuQuantum (stated goal)	Not achieved — hard block remains
cuQuantum restructuring	No regressions
Typo fixes	Not applied
Named constants (Device.cpu)	Not used

Bottom line: The PR makes a valid multi-GPU routing fix but should not be merged under its current title/description — it does not resolve issue #592. The description should be corrected to reflect the actual fix scope, and the cytnx_error_msg guard at line 1042 should either be resolved or tracked as a separate issue.

Review generated with Claude Code