refactor(linalg): implement Outer as Kron+reshape, retire Outer dispatch (#1003)#1105
Conversation
…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>
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
💡 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".
| return Kron(Tl, Tr).reshape(static_cast<cytnx_int64>(Tl.shape()[0]), | ||
| static_cast<cytnx_int64>(Tr.shape()[0])); |
There was a problem hiding this comment.
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 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Problem
Outer(a, b)for rank-1 vectors was implemented with its own per-dtype dispatch tables (Outer_iion CPU,cuOuter_iion GPU). BecauseOutercasts both operands to the promoted dtype before dispatching, only the diagonalOuter_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, soOuteron those dtypes dereferenced anullptrand 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*nvector whose element(i*n + j)isa_i * b_j. ReimplementOuterasKron(a, b).reshape({m, n})and delete theOuter_ii/cuOuter_iitables and their internal kernels.Kronalready 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 sameType.type_promoteresult dtype — so this both fixes the #1099 segfault and removes the dead dispatch machinery.cuKroncompiles under CUDA 13 (#999, closed), so the GPU path is covered.type_promote_gpu_tis intentionally left in place: it still underpins thetype_promote_from_gpu_pointer_ttrait thatcuKronuses, 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_oiitypedef,Outer_ii/cuOuter_iimembers + their population inlinalg_internal_interface.{hpp,cpp}, and the CMake source entries.Net: +216 / −3369.
Testing
Added
tests/linalg_test/Outer_test.cppandtests/gpu/linalg_test/Outer_test.cppwith independent hand-computed values: rectangular shape, single-element rank-1 operands,ComplexFloat × Double → ComplexDoublepromotion, and theInt16/Uint16/Booldiagonal cases that segfaulted (#1099).debug-openblas-cpu, ASan): all newOuter.*tests + existingDtypePromotion.OuterComplexfloatDoubleandZero_extentOuter cases pass.build_gpugpu_test_main, CUDA 13, RTX 4070 Ti): all 3Outer.*GPU tests pass.Refs #1003. Supersedes #1099 and #1101 (the missing dispatch rows no longer exist; there is no
cuOuterto add typed dispatch for).🤖 Generated with Claude Code