Use cuda::std::complex for typed GPU complex dispatch by ianmccul · Pull Request #1004 · Cytnx-dev/Cytnx

ianmccul · 2026-07-06T15:43:44Z

This is an alternative fix to #995.

use cuda::std::complex<float/double> as the typed GPU complex element type in Type_list_gpu
keep cuComplex / cuDoubleComplex only at explicit CUDA library ABI boundaries
teach Cytnx type traits about cuda::std::complex
update templated GPU Add/Sub/Mul/Div/Kron/Sort paths to use normal C++ complex arithmetic in the promoted output type

Fixes #994
Fixes #999
Supersedes #995

Motivation

cuComplex is a C ABI type, not a C++ arithmetic type. Using it as the element type in typed C++/CUDA templates makes mixed real/complex dispatch fragile and forces local helper functions for operations that should just be ordinary arithmetic.

cuda::std::complex is available in the CUDA toolchain already used by Cytnx and is the better type for typed CUDA kernel code. CUDA library calls that require cuComplex pointers can still cast explicitly at the call boundary.

Notes

This PR does not attempt to rewrite all older generated CUDA kernels or cuBLAS/cuSOLVER call sites. Those still use the existing cuComplex ABI where appropriate.

In particular, some large internal .cu kernel dispatch tables still cast void* buffers directly to cuComplex / cuDoubleComplex. This PR leaves those paths alone and only changes the typed GPU dispatch surface.

Tests

clang-format-14 -i <touched files>
git diff --check
cmake --build build/openblas-cuda --target cytnx -j 8
cmake --build build/openblas-cuda --target test_main -j 8
ctest --test-dir build/openblas-cpu --output-on-failure -j 8
- 1086/1086 passed
cmake --build build/openblas-cuda --target gpu_test_main -j 8
ctest --test-dir build/openblas-cuda -I 1087,1852 --output-on-failure -j 8
- 763/766 passed
- failures appear unrelated to this PR:
  - linalg_Test.gpu_BkFUt_Qr: GPU QR not implemented without cuQuantum
  - Gesvd_truncate.gpu_flag_combinations: cusolverDnDgesvdj execution failure in
    the real-double cuSOLVER path
  - Rsvd.gpu_flag_combinations: same real-double cuSOLVER path

gemini-code-assist · 2026-07-06T15:46:07Z

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

ianmccul · 2026-07-06T15:47:10Z

/gemini review
@codex review

gemini-code-assist

Code Review

This pull request replaces the legacy CUDA C-style complex types (cuDoubleComplex and cuComplex) with C++ standard-compliant cuda::std::complex types (cytnx_cuda_complex128 and cytnx_cuda_complex64) for GPU kernel code and storage representation. This change simplifies GPU kernel implementations by allowing standard arithmetic operators and member functions instead of legacy CUDA math APIs and custom conversion helpers. Additionally, static assertions have been added to verify the new type traits and promotions. There are no review comments, so I have no feedback to provide.

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 · 2026-07-06T15:51:28Z

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 7bbd8141f8

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

ianmccul · 2026-07-06T15:53:05Z

I have since reconfigured with CUQUANTUM and the previous failures now pass:

linalg_Test.gpu_BkFUt_Qr                Passed
Gesvd_truncate.gpu_flag_combinations    Passed
Rsvd.gpu_flag_combinations              Passed

codecov · 2026-07-07T00:59:11Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 30.92%. Comparing base (b5db6b8) to head (7bbd814).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1004   +/-   ##
=======================================
  Coverage   30.92%   30.92%           
=======================================
  Files         229      229           
  Lines       34760    34760           
  Branches    14398    14398           
=======================================
  Hits        10749    10749           
  Misses      16708    16708           
  Partials     7303     7303

Flag	Coverage Δ
cpp	`30.43% <ø> (ø)`
python	`59.41% <ø> (ø)`

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

Components	Coverage Δ
C++ backend	`32.25% <ø> (ø)`
Python bindings	`17.28% <ø> (ø)`
Python package	`59.41% <ø> (ø)`

Files with missing lines	Coverage Δ
include/Tensor.hpp	`53.05% <ø> (ø)`
include/Type.hpp	`51.21% <ø> (ø)`

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 b5db6b8...7bbd814. 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

The modified code LGTM.

@ianmccul

) Incorporate @ianmccul's review suggestion. Add a C++20 concept template <typename T> concept CytnxType = variant_contains_v<T, Type_list> && !std::is_void_v<T>; and constrain Storage_base::data<T>/at<T>/back<T> to it, with the unconstrained primary `= delete`d. Requesting an unsupported T (e.g. data<char>()) is now a hard compile error at the call site -- "use of deleted function" -- instead of relying on the downstream cy_typeid_v/type_spelling failures. This complements the existing type_spelling static_assert, which still guards a CytnxType added without a spelling. The GPU complex views are non-cytnx-dtype types, so they remain explicit specializations of the deleted primary. Following Ian's second note and #1004, data<> now offers both representations: the existing cuDoubleComplex / cuFloatComplex (cuBLAS/cuSOLVER ABI pointers) and the new cuda::std::complex<double/float> (what GPU kernels use internally). at<>/back<> gain no GPU specializations -- they were never valid for the cu* types. Verified: - Full CPU library + test suite build; Storage/Type/Tensor suites 300 passed, 0 failed (11 pre-existing skips). - data<char>() is rejected with "use of deleted function [with T = char]". - Storage_base.cpp compiles under UNI_GPU with the CCCL include path, so all four GPU data<> specializations (cuComplex + cuda::std::complex) type-check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

ianmccul · 2026-07-07T11:14:23Z

For bookkeeping: this should be treated as fixing #994 and #999, unless someone can reproduce after #1004. It also supersedes #995, which was the narrower Kron-only fix for the same cuFloatComplex * double -> cuFloatComplex failure mode.

…er) and delete cucomplex_arithmetic (#1019) * refactor(gpu): migrate Cpr to the cuda::std::complex typed dispatch path Stage 1 of completing #1004's GPU complex migration. Add/Sub/Mul/Div already route through the new cuda::std::complex typed dispatch (type_promote_gpu_t + TO(lhs) op TO(rhs) kernels); Cpr and Mod were the last consumers of the legacy cuAri_ii table (which forwards to the old cuComplex kernels via cuCadd/cuCmul and cucomplex_arithmetic's hand-rolled operators). Add cuCpr_dispatch (mirrors cuMul_dispatch) computing the comparison in the promoted type: out(bool) = (TO(lhs) == TO(rhs)) with TO = type_promote_gpu(TL, TR). Casting both operands to TO first is required because cuda::std::complex has no mixed-type operators (e.g. `complex<float> == double` is ill-formed), and it reproduces the old make_cuDoubleComplex(v, 0) widening for complex-vs- real comparisons. Rewire all 13 Cpr.cpp GPU call sites from lii.cuAri_ii[L][R](..., 4) to cuCpr_dispatch(...). The legacy cuCpr kernels stay for now (still reachable via the shared cuArithmetic_internal forwarder that Mod uses); they are removed once Mod is migrated and cuAri_ii is dropped. nvcc front-end verified: cuCpr_dispatch.cu, Cpr.cpp (GPU + CPU) all compile clean. Not runtime-tested (no GPU CI runner); mirrors the proven cuMul path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(gpu): migrate Mod to the cuda::std::complex typed dispatch path Stage 2. Add cuMod_dispatch (mirrors cuMul_dispatch), computing modulo in the promoted type TO = type_promote_gpu(TL, TR): integral -> TO(lhs) % TO(rhs), floating -> fmod/fmodf, matching the CPU ModOp<T>. Complex operands are rejected at the host dispatch ("[cuMod] Cannot mod complex numbers"), exactly as the old cuMod_internal_* functions and the CPU path do, so the kernel's complex branch is an unreachable if-constexpr stub. Rewire all 25 Mod.cpp GPU call sites (Tensor-Tensor, scalar-Tensor, Tensor-scalar) from lii.cuAri_ii[L][R](..., 5) to cuMod_dispatch(...). With Cpr (stage 1) and Mod migrated, nothing calls the legacy cuAri_ii table anymore; the old cuComplex kernels + cuArithmetic_internal layer become dead and are removed in the cleanup stage. nvcc front-end verified: cuMod_dispatch.cu, Mod.cpp (GPU + CPU) compile clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(gpu): migrate cuOuter to cuda::std::complex Stage 3. Switch cuOuter's generated dispatch instantiations from the cuComplex ABI types (cuDoubleComplex/cuFloatComplex) to cuda::std::complex (cytnx_cuda_complex128/64), and cast both operands to the output type T1 in cuOuter_kernel (T1(val) * T1(ptr)) since cuda::std::complex has no mixed-type operator*. This drops cuOuter's dependency on utils/cucomplex_arithmetic.hpp (removed the include). The 4 hand-added cuGer_internal (cuBLAS ger) fast paths for the cd/cf/d/f diagonals are preserved. Also update the generator's master type list (boostraps/cy_type.py cuDTYPES_FULL) to cuda::std::complex for consistency, so a future regeneration emits the correct types (the checked-in file additionally hand-uses cuGer for 4 BLAS-able cases, which the generator does not emit). nvcc front-end verified: cuOuter_internal.cu compiles clean. cuKron was already fully on cuda::std::complex (from #1004). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(gpu): delete cucomplex_arithmetic and the dead legacy cuCpr kernels Stage 4 (cleanup). With Cpr and Mod migrated to their typed dispatch paths and cuOuter on cuda::std::complex, the hand-rolled cuComplex operator overloads in utils/cucomplex_arithmetic.{hpp,cu} (operator== / operator* for mixed cuComplex/real) have exactly one remaining user: the dead legacy cuCpr_internal kernels, reachable only through the cuArithmetic_internal forwarders' type==4 branch (Cpr has no in-place variant, and out-of-place Cpr now uses cuCpr_dispatch, so type==4 is never dispatched). - Strip the 120 dead `else if (type == 4) -> cuCpr_internal_*` branches from cuArithmetic_internal.cu (type 4 now falls through to the existing else). - Delete cuCpr_internal.cu and its old per-type declarations (keep cuCpr_dispatch); drop it from CMakeLists. - Remove the now-unused cucomplex_arithmetic include from cuTrace_internal.cu, utils/utils.hpp, and utils_internal_interface.hpp. - Delete include/utils/cucomplex_arithmetic.hpp + src/utils/cucomplex_arithmetic.cu and their CMake entry. The remaining legacy arithmetic path (cuAri_ii -> cuArithmetic_internal -> cuAdd/Mul/Sub/Div/Mod_internal, using cuComplex.h builtins cuCadd/cuCmul, not these operators) stays: it is still used by the in-place ops iAdd/iSub/iMul/iDiv. That layer can be removed once those are migrated too. nvcc front-end verified: cuArithmetic_internal, cuMod/cuAdd/cuMul/cuOuter internals, Cpr.cpp (GPU+CPU) all compile clean; no cucomplex_arithmetic references remain in the tree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Ian McCulloch <ian@phys.nthu.edu.tw>

@gemini-code-assist

…e bodies (#979) * refactor(Storage): collapse at<T>/back<T>/data<T> into single template bodies Each of at<T>, back<T>, and data<T> was 11 hand-written explicit specializations with identical bodies, differing only in the dtype enum and the type spelling inside the error message. That copy-paste already let one dtype check drift out of sync (#965). Replace each family with one template body that derives the expected dtype from the compile-time trait Type_class::cy_typeid_v<T>, plus explicit instantiations for the same 11 types, so the exported symbols and header declarations are unchanged. The GPU-only data<cuDoubleComplex>/data<cuFloatComplex> specializations keep their extra device check and remain hand-written. Error-message wording is preserved byte-for-byte (verified by diffing the what() text of all 11 mismatch cases plus bounds/empty errors against the previous implementation). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(Storage): make type_spelling a hard compile error when unspecialized Address @gemini-code-assist review on #979. The primary template of type_spelling defaulted to nullptr, so adding a new at<T>/back<T>/data<T> instantiation without a matching spelling would compile and then feed a null pointer to the "%s" conversion in cytnx_error_msg — runtime UB. Route the primary template through a type_spelling_holder<T> whose body is static_assert(always_false_v<T>). Any type that reaches the dtype-mismatch message without an explicit spelling is now rejected at compile time. The 11 specialized types are unchanged, and the hand-written GPU data<cuDoubleComplex>/ data<cuFloatComplex> specializations never reference type_spelling, so they are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(Storage): constrain data/at/back with a CytnxType concept (#979) Incorporate @ianmccul's review suggestion. Add a C++20 concept template <typename T> concept CytnxType = variant_contains_v<T, Type_list> && !std::is_void_v<T>; and constrain Storage_base::data<T>/at<T>/back<T> to it, with the unconstrained primary `= delete`d. Requesting an unsupported T (e.g. data<char>()) is now a hard compile error at the call site -- "use of deleted function" -- instead of relying on the downstream cy_typeid_v/type_spelling failures. This complements the existing type_spelling static_assert, which still guards a CytnxType added without a spelling. The GPU complex views are non-cytnx-dtype types, so they remain explicit specializations of the deleted primary. Following Ian's second note and #1004, data<> now offers both representations: the existing cuDoubleComplex / cuFloatComplex (cuBLAS/cuSOLVER ABI pointers) and the new cuda::std::complex<double/float> (what GPU kernels use internally). at<>/back<> gain no GPU specializations -- they were never valid for the cu* types. Verified: - Full CPU library + test suite build; Storage/Type/Tensor suites 300 passed, 0 failed (11 pre-existing skips). - data<char>() is rejected with "use of deleted function [with T = char]". - Storage_base.cpp compiles under UNI_GPU with the CCCL include path, so all four GPU data<> specializations (cuComplex + cuda::std::complex) type-check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(Storage): constrain the Storage::data/at/back wrappers too Follow-up to constraining Storage_base. The thin Storage wrapper methods (Storage::data<T>/at<T>/back<T>, which just delegate to _impl) were still `template <class T>`, so an unsupported T errored one level deep inside the wrapper body instead of at the call site. Constrain all three to CytnxType so e.g. Storage::data<char>() fails at the call with "constraints not satisfied ... CytnxType<T>" naming the concept. They use CytnxType (not a wider GPU-inclusive concept) deliberately: the Storage_base GPU data<> specializations (cuComplex / cuda::std::complex) are defined only in Storage_base.cpp and are not declared in Storage.hpp, so they are not reachable through the wrapper from any external TU -- true on master as well. Advertising them in the wrapper constraint would let the constraint pass and then fail in the body at the deleted primary, which is worse than a clean call-site rejection. Making those views externally reachable (header-declaring the specializations) would be a separate change. Verified: full CPU library + test suite build; Storage/Tensor suites 280 passed / 0 failed (11 pre-existing skips); data<char>()/at<char>() rejected at the wrapper; header parses under UNI_GPU. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(Storage): make the GPU data<> pointer views reachable via the wrapper Declare the four GPU data<> explicit specializations (cuDoubleComplex, cuFloatComplex, cuda::std::complex<double>, cuda::std::complex<float>) in Storage.hpp, at namespace scope before the Storage wrapper. Previously they were defined only in Storage_base.cpp, so from any other TU `_impl->data<cuComplex>()` resolved to the deleted primary -- the views were unreachable outside that one file (true on master too, and there were no callers). With the declarations visible, widen the Storage::data<T> wrapper constraint back to StorageDataType (CytnxType plus the four GPU views), re-adding the GpuComplexView / StorageDataType concepts. So `storage.data<cuDoubleComplex>()` / `data<cuda::std::complex<double>>()` now compile and link end-to-end, while `data<char>()` is still rejected at the call site. at<>/back<> stay CytnxType (they have no GPU specializations). Verified: - GPU (UNI_GPU + CCCL include): the wrapper compiles data<cuDoubleComplex>, data<cuFloatComplex>, and both cuda::std::complex views; data<char> and at<cuDoubleComplex> are rejected; Storage_base.cpp emits all four specializations as strong (T) symbols matching the header declarations. - CPU: full library + test suite build; header parses (decls are #ifdef'd out); data<char> rejected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * build(cmake): export C++20 requirement to consumers via target_compile_features PR #979 adds the first C++20 concept (CytnxType) to a universally-included public header (Type.hpp). cytnx only sets C++20 at build level (CMAKE_CXX_STANDARD), which does not propagate through the installed/exported target, so find_package(cytnx) consumers compile the public headers in their default standard and fail to parse `concept` -- breaking DownstreamFindPackage. Add target_compile_features(cytnx PUBLIC cxx_std_20) so the exported target carries the requirement to consumers. This also closes a pre-existing latent gap: the public headers already require C++20 (std::format, std::source_location); the concept just makes it undeniable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Use cuda::std::complex for typed GPU complex dispatch

7bbd814

gemini-code-assist Bot reviewed Jul 6, 2026

View reviewed changes

yingjerkao approved these changes Jul 7, 2026

View reviewed changes

IvanaGyro merged commit 7b4d7dc into Cytnx-dev:master Jul 7, 2026
19 checks passed

This was referenced Jul 7, 2026

refactor(Storage): collapse at<T>/back<T>/data<T> into single template bodies #979

Merged

feat(pybind): register exception translator for cytnx::error -> CytnxError #989

Merged

yingjerkao mentioned this pull request Jul 9, 2026

refactor(gpu): finish the cuda::std::complex migration (Cpr/Mod/cuOuter) and delete cucomplex_arithmetic #1019

Merged

yingjerkao mentioned this pull request Jul 11, 2026

ci(gpu): native self-hosted GPU build + test workflow #1044

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use cuda::std::complex for typed GPU complex dispatch#1004

Use cuda::std::complex for typed GPU complex dispatch#1004
IvanaGyro merged 1 commit into
Cytnx-dev:masterfrom
ianmccul:codex/use-cuda-std-complex

ianmccul commented Jul 6, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot commented Jul 6, 2026

Uh oh!

ianmccul commented Jul 6, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

chatgpt-codex-connector Bot commented Jul 6, 2026

Uh oh!

ianmccul commented Jul 6, 2026

Uh oh!

codecov Bot commented Jul 7, 2026

Uh oh!

yingjerkao left a comment

Uh oh!

Uh oh!

ianmccul commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ianmccul commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Notes

Tests

Uh oh!

gemini-code-assist Bot commented Jul 6, 2026

Uh oh!

ianmccul commented Jul 6, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

chatgpt-codex-connector Bot commented Jul 6, 2026

Uh oh!

ianmccul commented Jul 6, 2026

Uh oh!

codecov Bot commented Jul 7, 2026

Codecov Report

Uh oh!

yingjerkao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ianmccul commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ianmccul commented Jul 6, 2026 •

edited

Loading