refactor(linalg): migrate GPU in-place arithmetic to typed dispatch, remove dead cuAri_ii table (#1003) by yingjerkao · Pull Request #1091 · Cytnx-dev/Cytnx

yingjerkao · 2026-07-19T10:14:47Z

Problem

Part of #1003 (consolidating CUDA elementwise arithmetic). Two coupled issues:

GPU in-place iAdd/iSub/iMul/iDiv still dispatched through the legacy lii.cuAri_ii 12×12 dtype-pair table — the last "ordinary arithmetic" path reaching it (step 8). That table's non-contiguous kernel writes out[idx] while out aliases the non-contiguous LHS storage and reads lhs[Lidx] — a read-at-Lidx/write-at-idx scatter that corrupts data (GPU: scalar in-place ops throw on non-contiguous tensors for *= and /=, and do a per-call H2D copy #988). iMul/iDiv guarded against it with a hard error; iAdd/iSub silently corrupted.
Once (1) is fixed, the entire cuAri_ii table and its generated dispatcher cuArithmetic_internal.{cu,hpp} become dead code (step 12).

Fix

Commit 1 — step 8 (typed in-place dispatch). Route the four in-place ops through the already-tested out-of-place cu{Add,Sub,Mul,Div}_dispatch kernels (from #1067), mirroring the CPU element-wise semantics (DispatchInplaceArithmeticCPU): in-place keeps the LHS dtype — compute in the promoted dtype, store into the LHS dtype. When the promoted dtype equals the LHS dtype the result is written straight into the LHS storage (preserves shared-storage aliasing); a broadcast scalar RHS is applied in place even for a non-contiguous LHS (layout-preserving, the #988 fix). Non-contiguous tensor⊗tensor in-place now uses the dispatch's mapper kernel and yields correct values.

Commit 2 — step 12 (remove the dead table). Delete cuArithmetic_internal.{cu,hpp} + its bet.py generator, drop the cuAri_ii member/population and the unused Arithmeticfunc_oii typedef from linalg_internal_interface.{hpp,cpp}, and include the six cu{Add,Sub,Mul,Div,Cpr,Mod}_internal.hpp headers directly (the umbrella header had transitively provided the live cu*_dispatch decls). Net −3413 lines.

Behavior notes (flagged for review)

A non-contiguous LHS with a tensor RHS becomes contiguous after the op (CPU keeps it strided). Values are correct; the old path corrupted/errored, so no working behavior regresses.
Mixed real/complex in-place computes in the promoted precision per the Merge CUDA unary and binary elementwise operations through one typed kernel framework #1003 spec, which can differ from CPU's std::complex::operator op= narrowing by ~float epsilon.

Full strided/precision parity would need a dedicated in-place kernel (writing lhs[Lidx]); noted as a possible follow-up.

Testing

On RTX 4070 Ti / CUDA 13:

64 existing in-place GPU tests (gpu_tensor_i{add,sub,mul,div}_*, all shapes × dtype-preserving pairs) — pass
new tests/gpu/linalg_test/InplaceArithmetic_test.cpp: dtype-widening, mixed complex/real, non-contiguous scalar, non-contiguous tensor, and independent hand-computed values — pass
out-of-place arithmetic sanity slice — pass
clang-format-14 clean; step 12 changes are #ifdef UNI_GPU-guarded so CPU builds are unaffected

Note: the repo has no automated GPU CI runner, so these GPU results are from a local run.

Advances #1003 (steps 8 and 12); does not close it.

🤖 Generated with Claude Code

#1003) Problem: GPU in-place iAdd/iSub/iMul/iDiv still dispatched through the legacy `lii.cuAri_ii` 12x12 dtype-pair table (#1003 step 8, the last "ordinary arithmetic" path reaching it). That table's non-contiguous kernel writes `out[idx]` while `out` aliases the non-contiguous LHS storage and reads `lhs[Lidx]` -- a read-at-Lidx / write-at-idx scatter that corrupts data (#988). iMul/iDiv guarded against it with a hard error; iAdd/iSub silently corrupted. Fix: reuse the typed out-of-place `cu{Add,Sub,Mul,Div}_dispatch` kernels (landed in #1067). In-place keeps the LHS dtype: compute in the promoted dtype and store into the LHS dtype, matching the CPU element-wise semantics (`DispatchInplaceArithmeticCPU`). When the promoted dtype equals the LHS dtype the result is written straight into the LHS storage (preserves shared-storage aliasing); a broadcast scalar RHS is applied in place even when the LHS is non-contiguous (layout-preserving, the #988 fix). Non-contiguous tensor<>tensor in-place now goes through the dispatch's mapper kernel and yields correct values in a contiguous LHS (previously corrupting/erroring). The legacy cuAri_ii table is now dead code; removing it is #1003 step 12. Behavior notes (flagged for review): (1) a non-contiguous LHS with a tensor RHS becomes contiguous after the op (CPU keeps it strided) -- values are correct and no working behavior is regressed. (2) mixed real/complex in-place computes in the promoted precision per #1003, which can differ from CPU's std::complex op= narrowing by ~float epsilon. Testing: on RTX 4070 Ti / CUDA 13, the 64 existing in-place GPU tests (Add/Sub/Mul/Div `gpu_tensor_i*_*`) pass, plus a new tests/gpu/linalg_test/InplaceArithmetic_test.cpp (dtype-widening, mixed complex/real, non-contiguous scalar, non-contiguous tensor, and independent hand-computed values). clang-format-14 clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

With GPU in-place arithmetic no longer routing through `lii.cuAri_ii` (previous commit), the legacy generated dtype-pair arithmetic dispatcher is unreachable. Remove it (#1003 step 12): - delete src/backend/linalg_internal_gpu/cuArithmetic_internal.{cu,hpp} and its bet.py generator; - drop the `cuAri_ii` member and its ~135-line population from linalg_internal_interface.{hpp,cpp}, plus the now-unused `Arithmeticfunc_oii` typedef; - the interface header previously pulled the live `cu*_dispatch` declarations in transitively through cuArithmetic_internal.hpp, so include the six cu{Add,Sub,Mul,Div,Cpr,Mod}_internal.hpp headers directly instead. The per-dtype-pair wrappers (cuAdd_internal_*, etc.) remain declared but are no longer referenced by any dispatch path, satisfying the #1003 criterion that the old generated CUDA arithmetic wrappers be removed or made unreachable. Testing: gpu_test_main builds clean; in-place and out-of-place arithmetic suites pass on RTX 4070 Ti / CUDA 13. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

gemini-code-assist

Code Review

This pull request refactors GPU in-place arithmetic operations (iAdd, iSub, iMul, and iDiv) by routing them through typed dispatch functions instead of the legacy cuAri_ii table, which enables proper dtype promotion and non-contiguous layout handling. It also introduces a new test suite to verify these paths. However, the reviewer identified critical bugs in all four arithmetic implementations when handling non-contiguous slices where the physical storage size exceeds the logical size. Specifically, the current implementation can cause shared-storage corruption by applying scalar operations to the entire physical storage, and it can trigger GPU buffer overflows by passing the larger physical storage size of the LHS to dispatch functions when writing to a smaller, contiguous temporary tensor. Actionable code suggestions were provided to fix these issues by ensuring physical size matches logical size before in-place scalar operations and using the correct logical size for the temporary tensor.

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.

gemini-code-assist · 2026-07-19T10:16:44Z

+          const unsigned int out_dtype = Type.type_promote(Lt.dtype(), Rt.dtype());
+          if (rhs_is_scalar) {
+            if (out_dtype == Lt.dtype()) {
+              linalg_internal::cuAdd_dispatch(
+                Lt._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl,
+                Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper);
+            } else {
+              Tensor out = Lt.astype(out_dtype);
+              linalg_internal::cuAdd_dispatch(
+                out._impl->storage()._impl, out._impl->storage()._impl, R._impl->storage()._impl,
+                Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper);
+              Lt = out.astype(Lt.dtype());
+            }
+          } else {
+            Tensor out(Lt.shape(), out_dtype, Lt.device());
+            linalg_internal::cuAdd_dispatch(out._impl->storage()._impl, Lt._impl->storage()._impl,
+                                            R._impl->storage()._impl,
+                                            Lt._impl->storage()._impl->size(), Lt._impl->shape(),
+                                            Lt._impl->invmapper(), Rt._impl->invmapper());
+            Lt = (out_dtype == Lt.dtype()) ? out : out.astype(Lt.dtype());
+          }


There are two critical bugs here when Lt is a non-contiguous slice (where the physical storage size Lt.storage().size() is larger than the logical size Lt.shape().size()):

Shared-Storage Corruption: If rhs_is_scalar is true and out_dtype == Lt.dtype(), calling cuAdd_dispatch with empty_mapper and the physical storage size Lt._impl->storage()._impl->size() will add the scalar to every element in the underlying physical storage, corrupting elements that are outside the slice Lt.

Buffer Overflow (Out-of-Bounds Write): In both the else branch of rhs_is_scalar and the tensor RHS branch, a contiguous out tensor of the logical size is allocated. However, passing Lt._impl->storage()._impl->size() (the physical storage size) as the size argument to cuAdd_dispatch will cause the kernel to write past the bounds of out's storage, leading to a critical GPU memory corruption or crash.

To fix these issues:

Only perform the physical in-place operation if the physical storage size equals the logical size (i.e., it is a permutation, not a slice).

Pass the logical size out._impl->storage()._impl->size() (or Lt.shape().size()) to cuAdd_dispatch when writing to the contiguous out tensor.

const unsigned int out_dtype = Type.type_promote(Lt.dtype(), Rt.dtype()); if (rhs_is_scalar) { if (out_dtype == Lt.dtype() && Lt.storage().size() == Lt.shape().size()) { linalg_internal::cuAdd_dispatch( Lt._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl, Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper); } else { Tensor out = Lt.astype(out_dtype); linalg_internal::cuAdd_dispatch( out._impl->storage()._impl, out._impl->storage()._impl, R._impl->storage()._impl, out._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper); Lt = out.astype(Lt.dtype()); } } else { Tensor out(Lt.shape(), out_dtype, Lt.device()); linalg_internal::cuAdd_dispatch(out._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl, out._impl->storage()._impl->size(), Lt._impl->shape(), Lt._impl->invmapper(), Rt._impl->invmapper()); Lt = (out_dtype == Lt.dtype()) ? out : out.astype(Lt.dtype()); }

gemini-code-assist · 2026-07-19T10:16:44Z

+          const unsigned int out_dtype = Type.type_promote(Lt.dtype(), Rt.dtype());
+          if (rhs_is_scalar) {
+            if (out_dtype == Lt.dtype()) {
+              linalg_internal::cuSub_dispatch(
+                Lt._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl,
+                Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper);
+            } else {
+              Tensor out = Lt.astype(out_dtype);
+              linalg_internal::cuSub_dispatch(
+                out._impl->storage()._impl, out._impl->storage()._impl, R._impl->storage()._impl,
+                Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper);
+              Lt = out.astype(Lt.dtype());
+            }
+          } else {
+            Tensor out(Lt.shape(), out_dtype, Lt.device());
+            linalg_internal::cuSub_dispatch(out._impl->storage()._impl, Lt._impl->storage()._impl,
+                                            R._impl->storage()._impl,
+                                            Lt._impl->storage()._impl->size(), Lt._impl->shape(),
+                                            Lt._impl->invmapper(), Rt._impl->invmapper());
+            Lt = (out_dtype == Lt.dtype()) ? out : out.astype(Lt.dtype());
+          }


There are two critical bugs here when Lt is a non-contiguous slice (where the physical storage size Lt.storage().size() is larger than the logical size Lt.shape().size()):

Shared-Storage Corruption: If rhs_is_scalar is true and out_dtype == Lt.dtype(), calling cuSub_dispatch with empty_mapper and the physical storage size Lt._impl->storage()._impl->size() will subtract the scalar from every element in the underlying physical storage, corrupting elements that are outside the slice Lt.

Buffer Overflow (Out-of-Bounds Write): In both the else branch of rhs_is_scalar and the tensor RHS branch, a contiguous out tensor of the logical size is allocated. However, passing Lt._impl->storage()._impl->size() (the physical storage size) as the size argument to cuSub_dispatch will cause the kernel to write past the bounds of out's storage, leading to a critical GPU memory corruption or crash.

To fix these issues:

Only perform the physical in-place operation if the physical storage size equals the logical size (i.e., it is a permutation, not a slice).

Pass the logical size out._impl->storage()._impl->size() (or Lt.shape().size()) to cuSub_dispatch when writing to the contiguous out tensor.

const unsigned int out_dtype = Type.type_promote(Lt.dtype(), Rt.dtype()); if (rhs_is_scalar) { if (out_dtype == Lt.dtype() && Lt.storage().size() == Lt.shape().size()) { linalg_internal::cuSub_dispatch( Lt._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl, Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper); } else { Tensor out = Lt.astype(out_dtype); linalg_internal::cuSub_dispatch( out._impl->storage()._impl, out._impl->storage()._impl, R._impl->storage()._impl, out._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper); Lt = out.astype(Lt.dtype()); } } else { Tensor out(Lt.shape(), out_dtype, Lt.device()); linalg_internal::cuSub_dispatch(out._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl, out._impl->storage()._impl->size(), Lt._impl->shape(), Lt._impl->invmapper(), Rt._impl->invmapper()); Lt = (out_dtype == Lt.dtype()) ? out : out.astype(Lt.dtype()); }

gemini-code-assist · 2026-07-19T10:16:45Z

+          const unsigned int out_dtype = Type.type_promote(Lt.dtype(), Rt.dtype());
          if (rhs_is_scalar) {
-            // Broadcast scalar RHS: the rconst kernel path scales every element of the
-            // (possibly non-contiguous) LHS storage in place, so the layout mappers are
-            // irrelevant and the result is correct. This is the #988 regression fix that lets
-            // e.g. `gpu_tensor.permute(...); gpu_tensor *= 2.0;` succeed again.
-            checkCudaErrors(cudaSetDevice(Lt.device()));
-            Tensor tmpo;
-            if (Lt.dtype() <= Rt.dtype())
-              tmpo = Lt;
-            else
-              tmpo = Lt.clone();
-            linalg_internal::lii.cuAri_ii[Lt.dtype()][Rt.dtype()](
-              tmpo._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl,
-              Lt._impl->storage()._impl->size(), Lt._impl->shape(), Lt._impl->invmapper(),
-              Rt._impl->invmapper(), 1);
-            if (Lt.dtype() > Rt.dtype()) Lt = tmpo;
+            if (out_dtype == Lt.dtype()) {
+              linalg_internal::cuMul_dispatch(
+                Lt._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl,
+                Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper);
+            } else {
+              Tensor out = Lt.astype(out_dtype);
+              linalg_internal::cuMul_dispatch(
+                out._impl->storage()._impl, out._impl->storage()._impl, R._impl->storage()._impl,
+                Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper);
+              Lt = out.astype(Lt.dtype());
+            }
          } else {
-            // Genuine non-contiguous tensor*=tensor: unlike cuAdd/cuSub, the cuMul GPU kernels
-            // ignore the layout mappers, so routing here would silently pair mismatched
-            // elements. Fail loudly instead of corrupting data. Call Contiguous_()/Contiguous()
-            // on the operands first.
-            cytnx_error_msg(true,
-                            "[iMul][on GPU/CUDA] non-contiguous tensor*=tensor is not supported. "
-                            "Call Contiguous_() or Contiguous() on the operands first%s",
-                            "\n");
+            Tensor out(Lt.shape(), out_dtype, Lt.device());
+            linalg_internal::cuMul_dispatch(out._impl->storage()._impl, Lt._impl->storage()._impl,
+                                            R._impl->storage()._impl,
+                                            Lt._impl->storage()._impl->size(), Lt._impl->shape(),
+                                            Lt._impl->invmapper(), Rt._impl->invmapper());
+            Lt = (out_dtype == Lt.dtype()) ? out : out.astype(Lt.dtype());
          }


There are two critical bugs here when Lt is a non-contiguous slice (where the physical storage size Lt.storage().size() is larger than the logical size Lt.shape().size()):

Shared-Storage Corruption: If rhs_is_scalar is true and out_dtype == Lt.dtype(), calling cuMul_dispatch with empty_mapper and the physical storage size Lt._impl->storage()._impl->size() will multiply every element in the underlying physical storage by the scalar, corrupting elements that are outside the slice Lt.

Buffer Overflow (Out-of-Bounds Write): In both the else branch of rhs_is_scalar and the tensor RHS branch, a contiguous out tensor of the logical size is allocated. However, passing Lt._impl->storage()._impl->size() (the physical storage size) as the size argument to cuMul_dispatch will cause the kernel to write past the bounds of out's storage, leading to a critical GPU memory corruption or crash.

To fix these issues:

Only perform the physical in-place operation if the physical storage size equals the logical size (i.e., it is a permutation, not a slice).

Pass the logical size out._impl->storage()._impl->size() (or Lt.shape().size()) to cuMul_dispatch when writing to the contiguous out tensor.

const unsigned int out_dtype = Type.type_promote(Lt.dtype(), Rt.dtype()); if (rhs_is_scalar) { if (out_dtype == Lt.dtype() && Lt.storage().size() == Lt.shape().size()) { linalg_internal::cuMul_dispatch( Lt._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl, Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper); } else { Tensor out = Lt.astype(out_dtype); linalg_internal::cuMul_dispatch( out._impl->storage()._impl, out._impl->storage()._impl, R._impl->storage()._impl, out._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper); Lt = out.astype(Lt.dtype()); } } else { Tensor out(Lt.shape(), out_dtype, Lt.device()); linalg_internal::cuMul_dispatch(out._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl, out._impl->storage()._impl->size(), Lt._impl->shape(), Lt._impl->invmapper(), Rt._impl->invmapper()); Lt = (out_dtype == Lt.dtype()) ? out : out.astype(Lt.dtype()); }

gemini-code-assist · 2026-07-19T10:16:45Z

+          const unsigned int out_dtype = Type.type_promote(Lt.dtype(), Rt.dtype());
          if (rhs_is_scalar) {
-            // Broadcast scalar RHS: the rconst kernel path divides every element of the
-            // (possibly non-contiguous) LHS storage by the scalar in place, so the layout
-            // mappers are irrelevant and the result is correct. This is the #988 regression fix
-            // that lets e.g. `gpu_tensor.permute(...); gpu_tensor /= 2.0;` succeed again.
-            checkCudaErrors(cudaSetDevice(Lt.device()));
-            Tensor tmpo;
-            if (Lt.dtype() <= Rt.dtype())
-              tmpo = Lt;
-            else
-              tmpo = Lt.clone();
-            linalg_internal::lii.cuAri_ii[Lt.dtype()][Rt.dtype()](
-              tmpo._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl,
-              Lt._impl->storage()._impl->size(), Lt._impl->shape(), Lt._impl->invmapper(),
-              Rt._impl->invmapper(), 3);
-            if (Lt.dtype() > Rt.dtype()) Lt = tmpo;
+            if (out_dtype == Lt.dtype()) {
+              linalg_internal::cuDiv_dispatch(
+                Lt._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl,
+                Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper);
+            } else {
+              Tensor out = Lt.astype(out_dtype);
+              linalg_internal::cuDiv_dispatch(
+                out._impl->storage()._impl, out._impl->storage()._impl, R._impl->storage()._impl,
+                Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper);
+              Lt = out.astype(Lt.dtype());
+            }
          } else {
-            // Genuine non-contiguous tensor/=tensor: unlike cuAdd/cuSub, the cuDiv GPU kernels
-            // ignore the layout mappers, so routing here would silently pair mismatched
-            // elements. Fail loudly instead of corrupting data. Call Contiguous_()/Contiguous()
-            // on the operands first.
-            cytnx_error_msg(true,
-                            "[iDiv][on GPU/CUDA] non-contiguous tensor/=tensor is not supported. "
-                            "Call Contiguous_() or Contiguous() on the operands first%s",
-                            "\n");
+            Tensor out(Lt.shape(), out_dtype, Lt.device());
+            linalg_internal::cuDiv_dispatch(out._impl->storage()._impl, Lt._impl->storage()._impl,
+                                            R._impl->storage()._impl,
+                                            Lt._impl->storage()._impl->size(), Lt._impl->shape(),
+                                            Lt._impl->invmapper(), Rt._impl->invmapper());
+            Lt = (out_dtype == Lt.dtype()) ? out : out.astype(Lt.dtype());
          }


There are two critical bugs here when Lt is a non-contiguous slice (where the physical storage size Lt.storage().size() is larger than the logical size Lt.shape().size()):

Shared-Storage Corruption: If rhs_is_scalar is true and out_dtype == Lt.dtype(), calling cuDiv_dispatch with empty_mapper and the physical storage size Lt._impl->storage()._impl->size() will divide every element in the underlying physical storage by the scalar, corrupting elements that are outside the slice Lt.

Buffer Overflow (Out-of-Bounds Write): In both the else branch of rhs_is_scalar and the tensor RHS branch, a contiguous out tensor of the logical size is allocated. However, passing Lt._impl->storage()._impl->size() (the physical storage size) as the size argument to cuDiv_dispatch will cause the kernel to write past the bounds of out's storage, leading to a critical GPU memory corruption or crash.

To fix these issues:

Only perform the physical in-place operation if the physical storage size equals the logical size (i.e., it is a permutation, not a slice).

Pass the logical size out._impl->storage()._impl->size() (or Lt.shape().size()) to cuDiv_dispatch when writing to the contiguous out tensor.

const unsigned int out_dtype = Type.type_promote(Lt.dtype(), Rt.dtype()); if (rhs_is_scalar) { if (out_dtype == Lt.dtype() && Lt.storage().size() == Lt.shape().size()) { linalg_internal::cuDiv_dispatch( Lt._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl, Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper); } else { Tensor out = Lt.astype(out_dtype); linalg_internal::cuDiv_dispatch( out._impl->storage()._impl, out._impl->storage()._impl, R._impl->storage()._impl, out._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper); Lt = out.astype(Lt.dtype()); } } else { Tensor out(Lt.shape(), out_dtype, Lt.device()); linalg_internal::cuDiv_dispatch(out._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl, out._impl->storage()._impl->size(), Lt._impl->shape(), Lt._impl->invmapper(), Rt._impl->invmapper()); Lt = (out_dtype == Lt.dtype()) ? out : out.astype(Lt.dtype()); }

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1992dcf7bb

ℹ️ 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 · 2026-07-19T10:19:37Z

+            linalg_internal::cuAdd_dispatch(
+              out._impl->storage()._impl, Lt._impl->storage()._impl, R._impl->storage()._impl,
+              Lt._impl->storage()._impl->size(), empty_mapper, empty_mapper, empty_mapper);
+            Lt = out.astype(Lt.dtype());


Preserve shared storage for promoted in-place ops

When the promoted dtype differs from the LHS dtype in a GPU in-place op (for example ComplexFloat LHS with a Double tensor RHS, which this change explicitly adds), this assignment replaces Lt with a new tensor instead of writing back into the original storage. Any existing alias or permuted view that shares the original LHS storage is left unchanged, unlike the CPU iAdd path and the documented in-place contract that the left tensor is modified; the same pattern appears in the other in-place arithmetic files, so promoted GPU tensor-RHS ops silently detach shared data.

Useful? React with 👍 / 👎.

IvanaGyro · 2026-07-19T10:47:44Z

+  // Relative element-wise comparison. Used for mixed real/complex in-place: the GPU computes in the
+  // promoted dtype (per #1003) then narrows to the LHS dtype, while CPU's std::complex op= narrows
+  // the RHS to the LHS scalar type first, so the two agree only to float rounding.
+  bool NearlyEqRel(const Tensor& a_dev, const Tensor& b_dev, double rel_tol) {


It seems that there is a pre-defined tool in gpu_teset_tool.h. And by the coding style we defined in the skill, the function name should be snake case. Additionally, the test name should not use _

IvanaGyro · 2026-07-19T10:49:38Z

+#include "gtest/gtest.h"
+
+#include "gpu_test_tools.h"
+#include "cytnx.hpp"


Wrong include order.

yingjerkao · 2026-07-19T16:10:02Z

Superseded by master. Since this PR was opened, master absorbed the same #1003 work through two merges:

tests: reorganize and modernize unit tests (namespaces, headers, formatting) #1080 already removed the dead cuAri_ii table, the Arithmeticfunc_oii typedef, and swapped the umbrella cuArithmetic_internal.hpp include for the six individual cu{Add,Sub,Mul,Div,Mod,Cpr}_internal.hpp headers — i.e. this PR's "remove dead table" commit.
Typed Storage dispatch: GPU conversion #1013 / refactor(linalg)!: GPU elementwise arithmetic typed std::visit dispatch (#1013) #1067 migrated the GPU in-place path to a dedicated cuiArithmeticDispatch in-place kernel that applies the layout mappers and writes through in place. That is strictly better than this PR's approach (which reused the out-of-place cu*_dispatch and therefore produced a contiguous result for non-contiguous tensor (op)= tensor, diverging from the CPU strided-write-through — the tradeoff flagged in this PR's description). Master now preserves strided layout with full CPU parity.

Test coverage is also covered upstream: master's tests/gpu/linalg_test/InplacePromote_test.cpp exercises the promoting + non-contiguous in-place cases (and validates the strided behavior this PR could not provide). Closing as fully superseded; no salvage needed.

yingjerkao and others added 2 commits July 19, 2026 12:18

gemini-code-assist Bot reviewed Jul 19, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 19, 2026

View reviewed changes

IvanaGyro reviewed Jul 19, 2026

View reviewed changes

This was referenced Jul 19, 2026

refactor(linalg): constrain free Tensor/UniTensor scalar operators to scalar-like types (#1003) #1093

Open

refactor(linalg): typed GPU dispatch for unary ops Abs/Exp/Pow/Conj (#1003 step 11) #1094

Open

yingjerkao closed this Jul 19, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(linalg): migrate GPU in-place arithmetic to typed dispatch, remove dead cuAri_ii table (#1003)#1091

refactor(linalg): migrate GPU in-place arithmetic to typed dispatch, remove dead cuAri_ii table (#1003)#1091
yingjerkao wants to merge 2 commits into
masterfrom
refactor/1003-gpu-inplace-typed-dispatch

yingjerkao commented Jul 19, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 19, 2026

Uh oh!

IvanaGyro Jul 19, 2026

Uh oh!

IvanaGyro Jul 19, 2026

Uh oh!

yingjerkao commented Jul 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

yingjerkao commented Jul 19, 2026

Problem

Fix

Behavior notes (flagged for review)

Testing

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

IvanaGyro Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

IvanaGyro Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

yingjerkao commented Jul 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants