Skip to content

refactor(linalg): implement Outer as Kron+reshape, retire Outer dispatch (#1003)#1105

Merged
yingjerkao merged 1 commit into
masterfrom
refactor/1003-outer-via-kron
Jul 21, 2026
Merged

refactor(linalg): implement Outer as Kron+reshape, retire Outer dispatch (#1003)#1105
yingjerkao merged 1 commit into
masterfrom
refactor/1003-outer-via-kron

Conversation

@yingjerkao

Copy link
Copy Markdown
Collaborator

Problem

Outer(a, b) for rank-1 vectors was implemented with its own per-dtype dispatch tables (Outer_ii on CPU, cuOuter_ii on GPU). Because Outer casts both operands to the promoted dtype before dispatching, only the diagonal Outer_ii[t][t] entries were ever reachable — the ~30 off-diagonal entries were dead code, and three diagonal rows (Int16, Uint16, Bool) were never registered, so Outer on those dtypes dereferenced a nullptr and segfaulted (#1099).

Fix

Per @ianmccul's review of #1099/#1101: Outer(a, b) is exactly the Kronecker product of the two vectors reshaped to {m, n}Kron(a, b) is a length-m*n vector whose element (i*n + j) is a_i * b_j. Reimplement Outer as Kron(a, b).reshape({m, n}) and delete the Outer_ii / cuOuter_ii tables and their internal kernels.

Kron already uses variant-based typed dispatch (type_promote_from_pointer_t / type_promote_from_gpu_pointer_t) covering every dtype pair on both CPU and GPU, and shares the same Type.type_promote result dtype — so this both fixes the #1099 segfault and removes the dead dispatch machinery. cuKron compiles under CUDA 13 (#999, closed), so the GPU path is covered.

type_promote_gpu_t is intentionally left in place: it still underpins the type_promote_from_gpu_pointer_t trait that cuKron uses, so it is not dead (this is why the Type_test.cpp break flagged on #1101 does not apply here).

Removed

  • src/backend/linalg_internal_cpu/Outer_internal.{cpp,hpp}
  • src/backend/linalg_internal_gpu/cuOuter_internal.{cu,hpp}
  • Outerfunc_oii typedef, Outer_ii / cuOuter_ii members + their population in linalg_internal_interface.{hpp,cpp}, and the CMake source entries.

Net: +216 / −3369.

Testing

Added tests/linalg_test/Outer_test.cpp and tests/gpu/linalg_test/Outer_test.cpp with independent hand-computed values: rectangular shape, single-element rank-1 operands, ComplexFloat × Double → ComplexDouble promotion, and the Int16/Uint16/Bool diagonal cases that segfaulted (#1099).

  • CPU (debug-openblas-cpu, ASan): all new Outer.* tests + existing DtypePromotion.OuterComplexfloatDouble and Zero_extent Outer cases pass.
  • GPU (build_gpu gpu_test_main, CUDA 13, RTX 4070 Ti): all 3 Outer.* GPU tests pass.

Refs #1003. Supersedes #1099 and #1101 (the missing dispatch rows no longer exist; there is no cuOuter to add typed dispatch for).

🤖 Generated with Claude Code

…tch (#1003)

Problem: Outer(a, b) for rank-1 vectors was implemented with its own per-dtype
dispatch tables (Outer_ii on CPU, cuOuter_ii on GPU). Because Outer casts both
operands to the promoted dtype before dispatching, only the diagonal
Outer_ii[t][t] entries were ever reachable -- the ~30 off-diagonal entries were
dead code, and three diagonal rows (Int16, Uint16, Bool) were never registered,
so Outer on those dtypes dereferenced a nullptr and segfaulted (#1099).

Fix (per @ianmccul's review of #1099/#1101): Outer(a, b) is exactly the
Kronecker product of the two vectors reshaped to {m, n} -- Kron(a, b) is a
length-(m*n) vector whose element (i*n + j) is a_i * b_j. Reimplement Outer as
`Kron(a, b).reshape({m, n})` and delete the Outer_ii / cuOuter_ii tables and
their internal kernels. Kron already uses variant-based typed dispatch
(type_promote_from_pointer_t / type_promote_from_gpu_pointer_t) that covers
every dtype pair on both CPU and GPU and shares the same Type.type_promote
result dtype, so this both fixes the #1099 segfault and removes the dead
dispatch machinery. cuKron compiles under CUDA 13 (#999, closed), so the GPU
path is covered too.

This supersedes #1099 (the missing Int16/Uint16/Bool rows no longer exist) and
the "typed GPU dispatch for Outer" half of #1101 (there is no cuOuter to
dispatch). type_promote_gpu_t is left in place: it still underpins the
type_promote_from_gpu_pointer_t trait that cuKron uses, so it is not dead.

Removed:
- src/backend/linalg_internal_cpu/Outer_internal.{cpp,hpp}
- src/backend/linalg_internal_gpu/cuOuter_internal.{cu,hpp}
- Outerfunc_oii typedef, Outer_ii / cuOuter_ii members and their population in
  linalg_internal_interface.{hpp,cpp}, plus the CMake source entries.

Testing: added tests/linalg_test/Outer_test.cpp and
tests/gpu/linalg_test/Outer_test.cpp with independent hand-computed values --
rectangular shape, single-element rank-1 operands, ComplexFloat x Double ->
ComplexDouble promotion, and the Int16/Uint16/Bool diagonal cases that
segfaulted (#1099). CPU: debug-openblas-cpu ASan build, all Outer + existing
DtypePromotion/Zero_extent Outer tests pass. GPU: build_gpu gpu_test_main
(CUDA 13) build + Outer GPU tests pass.

Refs #1003. Supersedes #1099, #1101.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the Outer product implementation by retiring the legacy per-dtype Outer_ii and cuOuter_ii dispatch tables on both CPU and GPU, and instead delegating the operation to Kron(Tl, Tr).reshape(...). This change leverages Kron's robust variant-based typed dispatch, resolving a segfault issue (#1099) when handling Int16, Uint16, and Bool dtypes. Comprehensive CPU and GPU unit tests have been added to verify the correctness of the new implementation across various shapes and dtype promotions. I have no feedback to provide as there are no review comments.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6d76c960c

ℹ️ 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".

Comment thread src/linalg/Outer.cpp
Comment on lines +28 to +29
return Kron(Tl, Tr).reshape(static_cast<cytnx_int64>(Tl.shape()[0]),
static_cast<cytnx_int64>(Tr.shape()[0]));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve promoted precision in CPU Outer

On CPU mixed ComplexFloat/Double inputs, this now routes through Kron with the original input dtypes, but the CPU Kron kernel multiplies Lin[x] * Rin[y]; Cytnx's overload for cytnx_complex64 * cytnx_double returns cytnx_complex64. That means values such as Outer(complex64(1, 0), double(1.0 + 2^-40)) are rounded to float before being stored in the ComplexDouble output, whereas the removed Outer path cast both operands to the promoted dtype first. Please cast both operands to Type.type_promote(...) before calling Kron, or make the Kron multiply happen in the output type.

Useful? React with 👍 / 👎.

@ianmccul ianmccul left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.25%. Comparing base (014a386) to head (f6d76c9).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1105      +/-   ##
==========================================
+ Coverage   72.71%   73.25%   +0.53%     
==========================================
  Files         226      225       -1     
  Lines       28151    27802     -349     
  Branches       71       71              
==========================================
- Hits        20471    20365     -106     
+ Misses       7659     7416     -243     
  Partials       21       21              
Flag Coverage Δ
cpp 73.37% <100.00%> (+0.54%) ⬆️
python 64.13% <ø> (ø)

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

Components Coverage Δ
C++ backend 72.56% <100.00%> (+0.62%) ⬆️
Python bindings 77.70% <ø> (ø)
Python package 64.13% <ø> (ø)
Files with missing lines Coverage Δ
src/backend/linalg_internal_interface.cpp 100.00% <ø> (ø)
src/linalg/Outer.cpp 100.00% <100.00%> (+5.26%) ⬆️

... and 1 file 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 014a386...f6d76c9. 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.

@yingjerkao
yingjerkao merged commit 381aa31 into master Jul 21, 2026
19 checks passed
@yingjerkao
yingjerkao deleted the refactor/1003-outer-via-kron branch July 21, 2026 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants