refactor(utils)!: drop namespace-scope complex/builtin operator overloads (#1003)

[yingjerkao]

Problem

include/utils/complex_arithmetic.hpp declared ~300 namespace-scope operators (+ - * / ==)
between cytnx_complex64/128 and every builtin scalar, implemented in a 638-line .cpp.
Because cytnx_complex64/128 are aliases for std::complex<float/double> and builtins convert
into them, these are effectively Cytnx operators over standard-library and builtin types: under
using namespace cytnx they enter ordinary unqualified lookup for completely unrelated code,
amplified by C++20's reversed operator== rewrite. The reported symptom (#1003):

#include <vector>
#include "linalg.hpp"
using namespace cytnx;

int main() {
  std::vector<bool> values(1);
  bool rhs = true;
  return values[0] == rhs ? 0 : 1;   // error: ambiguous overload for 'operator=='
}

The header is pulled in by the umbrella utils.hpp, so the operators were in scope
codebase-wide and for any downstream using namespace cytnx.

Fix

Replace the whole hand-written surface with five constrained templates in the same header
(the .cpp is deleted). This is the alternative @ianmccul raised in review — tighten the
declarations rather than delete the functionality:

template <class L, class R>
concept ComplexMixedOperands = CytnxType<L> && CytnxType<R> &&
  (is_complex_floating_point_v<L> || is_complex_floating_point_v<R>) &&
  !internal::std_complex_handles_v<L, R>;

template <class L, class R>
  requires ComplexMixedOperands<L, R>
constexpr auto operator*(L lhs, R rhs) {
  using TO = Type_class::type_promote_t<L, R>;
  return static_cast<TO>(lhs) * static_cast<TO>(rhs);
}

Three conditions do the work:

• CytnxType<L> && CytnxType<R> — both operands must be members of Type_list. This is what
  keeps std::vector<bool>::reference, user-defined classes, char, long double, … from ever
  forming a candidate. It also guarantees type_promote_t is well-formed, so the constraint is
  checked before any hard error can escape.
• at least one complex operand — the pollution class is gone by construction.
• !std_complex_handles_v — pairs std::complex already provides an operator for
  (complex<T> with complex<T> or with T) stay with std, so the cytnx template can never
  become a second equally-good candidate and make them ambiguous.

One operator== suffices: C++20 synthesizes scalar == complex from the reversed candidate and
!= from the negation.

Also in this PR:

• Kron_general now computes static_cast<TO>(Lin[x]) * static_cast<TO>(Rin[y]) —
  explicit output-type arithmetic per the Merge CUDA unary and binary elementwise operations through one typed kernel framework #1003 rule, rather than relying on whichever operator
  C++ finds for the raw operand pair. (The earlier if constexpr (requires …) variant is gone;
  see the review thread.)
• physics::spin uses std::sqrt instead of pow(x, 0.5) and builds the 'y' component's
  imaginary entry directly.
• developer_tools/Makefile no longer references the deleted complex_arithmetic object.

Breaking change

The ! is for a type-promotion change, not a removal. Mixed complex/real results now fold
through Type.type_promote, which crosses the real/complex boundary by precision (#858, #982):

expression	before	after
complex64 * double	complex64 (precision silently dropped)	complex128
complex64 + double	complex64	complex128
complex128 * complex64	complex128	complex128 (unchanged)
complex128 * int	complex128	complex128 (unchanged)

Everything Codex and @ianmccul listed as a would-be source break still compiles:
z * 2, z == 0, z + cytnx_float{1}, w * cytnx_double{2}.

The only removals are the UNI_GPU cuDoubleComplex/cuFloatComplex operator== declarations,
which had no call sites (#1019 owns the CUDA-side migration).

Testing

• tests/overload_hygiene_test.cpp — the std::vector<bool>::reference == bool guard (a
  compile-time guard; the vector is deliberately non-const, since a const vector<bool> yields a
  plain bool and would guard nothing), the concept's admit/reject set, and the mixed
  complex/scalar policy: result dtypes via static_assert plus exact values for + - * / ==
  including negative, fractional, unsigned, bool and cross-precision operands.
  Verified to fail to compile on pre-fix master (5 ambiguous overload for 'operator=='
  errors, GCC 13.3, C++20).
• tests/Physics_test.cpp — spin-1/2 and spin-1 x/y/z matrices against the textbook
  ħ = 1 values, plus an independent algebraic check that [Sx, Sy] = i Sz for S = ½, 1, 3/2, 2,
  and the rejection of non-half-integer S.
• tests/linalg_test/Kron_test.cpp — mixed-precision ComplexDouble × ComplexFloat,
  ComplexFloat × Double (the promotion that crosses the real/complex boundary), complex ×
  signed integer, Uint32 × Int64, and a same-dtype control, all against hand-computed values.

Gates run locally on this rebase:

• debug-openblas-cpu (Debug + ASan): 1836/1836 ctest tests pass.
• pytest pytests/: 255 passed, 1 skipped.
• Python bindings (pycytnx) build and import; cytnx.physics.spin(0.5, 'y') is correct.
• CUDA (USE_CUDA=ON) build of the library and test_main.
• pre-commit run clean (clang-format v14).

Advances #1003 (Ian's operator-hygiene fold-in).

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request removes the namespace-scope complex-to-builtin operator overloads in utils/complex_arithmetic.hpp and utils/complex_arithmetic.cpp to prevent them from leaking into ordinary overload resolution (resolving issue #1003). It also introduces a regression test overload_hygiene_test.cpp to guard against this. To support this removal, several files were updated to avoid relying on these operators, such as explicitly constructing complex numbers in src/Physics.cpp and casting operands in Kron_internal.hpp and Outer_internal.cpp.

Feedback on the changes:

• In Kron_internal.hpp and Outer_internal.cpp, casting both operands to the output type TO before multiplication can be inefficient (e.g., forcing complex multiplication on real inputs). It is recommended to use C++20 if constexpr with a requires clause to multiply directly when possible.
• In src/Physics.cpp, using pow(..., 0.5) and explicit cytnx_double casts should be replaced with std::sqrt and standard double types to improve efficiency and align with the repository style guide.

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.

[IvanaGyro]

The tests need to follow the style landed by #1080, and should not use _ as the test name.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.05%. Comparing base (2318026) to head (83d6120).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1092      +/-   ##
==========================================
+ Coverage   73.06%   74.05%   +0.99%     
==========================================
  Files         224      224              
  Lines       27642    27274     -368     
  Branches       71       71              
==========================================
+ Hits        20196    20198       +2     
+ Misses       7425     7055     -370     
  Partials       21       21              

Flag	Coverage Δ
cpp	74.19% <100.00%> (+1.00%)	⬆️
python	64.13% <ø> (ø)

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

Components	Coverage Δ
C++ backend	73.72% <100.00%> (+1.18%)	⬆️
Python bindings	76.75% <ø> (ø)
Python package	64.13% <ø> (ø)

Files with missing lines	Coverage Δ
include/utils/complex_arithmetic.hpp	100.00% <100.00%> (ø)
src/Physics.cpp	29.54% <100.00%> (+1.51%)	⬆️
src/backend/linalg_internal_cpu/Kron_internal.hpp	100.00% <100.00%> (ø)

... 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 2318026...83d6120. 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]

Thanks — all addressed in the latest commit:

• Kron_internal / Outer_internal (Gemini, avoid forced complex multiply): adopted the if constexpr approach, but guarded on the full cast expression rather than a bare product:

  if constexpr (requires(TL l, TR r) { static_cast<TO>(l * r); }) {
    out[i] = static_cast<TO>(Lin[x] * Rin[y]);   // e.g. real*real -> real, widened to TO
  } else {
    out[i] = static_cast<TO>(Lin[x]) * static_cast<TO>(Rin[y]);
  }

  A bare requires { l * r; } (as suggested) is not safe in Cytnx: cytnx::Scalar's implicit conversions make l * r well-formed for mixed complex/real pairs, but the result is a cytnx::Scalar that static_cast<std::complex<...>> can't accept — so the direct path would be taken and then fail to compile. Guarding on static_cast<TO>(l * r) selects the direct product only when the whole expression is well-formed.

• Physics::spin (Gemini): std::sqrt instead of pow(..., 0.5), dropped the redundant cytnx_double cast.

• Test naming (@IvanaGyro): renamed the OverloadHygiene cases to PascalCase without underscores. If there are other #1080 style aspects you'd like followed here (namespacing, etc.), let me know and I'll apply them.

[yingjerkao]

Review status (bot threads):

• src/Physics.cpp pow(x, 0.5) → std::sqrt (@gemini-code-assist): fixed in 41070f4 (Sx now matches the Sy elements, which already used std::sqrt).
• Kron_internal.hpp / Outer_internal.cpp cast-both-to-TO inefficiency (@gemini-code-assist): already addressed — the current kernel multiplies in the native operand types and casts only the result (if constexpr (requires { static_cast<TO>(l * r); }) out[i] = static_cast<TO>(Lin[x] * Rin[y]);), casting both operands only in the fallback. (Also note Outer_internal.cpp is deleted entirely by refactor(linalg): implement Outer as Kron+reshape, retire Outer dispatch (#1003) #1105.)

These threads look ready to resolve.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 41070f4b6e

ℹ️ 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]

Codex review notes:

Deleting the 946-line handwritten overload layer is directionally good, but I think this still needs changes before merge.

1. The API break is substantially understated. In C++20, the std::complex scalar operator templates require the scalar to deduce as the same underlying type. Removing this header therefore breaks ordinary expressions such as:

cytnx_complex128 z;
z * 2;                 // int
z == 0;                // int
z + cytnx_float{1};    // float
cytnx_complex64 w;
w * cytnx_double{2};   // double

It is not limited to ComplexDouble/ComplexFloat combinations. The existing Codex thread is correct. Deleting these operations may still be an acceptable Cytnx 2.0 decision, but the PR needs to describe the real source break. Alternatively, the hundreds of concrete overloads could be replaced by a handful of tightly constrained templates requiring at least one exactly deduced complex operand. That would preserve mixed arithmetic without allowing unrelated types into overload resolution.

2. The Gemini-inspired requires branch in Kron is misguided.

if constexpr (requires { static_cast<TO>(l * r); })

TO is derived from type_promote_t<TL, TR>. If both inputs are real, TO is real, so the supposed "two real inputs but complex output" optimization cannot arise. More importantly, evaluating l * r before conversion uses C++'s native promotion rules rather than Cytnx's selected output type. That is exactly what the typed-dispatch work is intended to prevent, particularly for signed/unsigned combinations. This should simply be:

out[i] = static_cast<TO>(Lin[x]) * static_cast<TO>(Rin[y]);

3. developer_tools/Makefile is broken. It still includes complex_arithmetic.o and has a rule depending on the deleted .cpp. The CMake source list was updated, but this build path was not.

4. The branch is now 158 commits behind and conflicting. Outer_internal.cpp has already been removed by refactor(linalg): implement Outer as Kron+reshape, retire Outer dispatch (#1003) #1105, so that portion of the patch should disappear during the rebase. The previous green checks are against a very old tree. The red macOS job did not expose a platform defect; it failed while trying to merge the current target branch.

5. The tests cover the pollution symptom, but not the changed complex API. The vector<bool> regression is useful. Tests should also pin the chosen policy for mixed complex/scalar expressions and commit the claimed Physics::spin and mixed-precision Kron correctness checks rather than mentioning only an external harness.

After rebasing, the clean version should be considerably smaller: remove the old implementation and include sites, update the Makefile, use explicit output-type arithmetic in Kron, retain the Physics cleanup, and clearly decide whether mixed complex arithmetic is deliberately removed or replaced with constrained templates.

[yingjerkao]

@ianmccul — thanks, all five points are addressed. The branch is rebased onto current master
and squashed to a single commit; the PR body is rewritten.

1. The API break — took the constrained-template option.

You were right that the break was understated: z * 2, z == 0, z + cytnx_float{1} and
w * cytnx_double{2} all lost their operator, not just complex128 × complex64. Rather than
document a break that wide, I took your alternative and replaced the ~300 declarations with five
constrained templates:

template <class L, class R>
concept ComplexMixedOperands = CytnxType<L> && CytnxType<R> &&
  (is_complex_floating_point_v<L> || is_complex_floating_point_v<R>) &&
  !internal::std_complex_handles_v<L, R>;

Three conditions carry it: CytnxType on both sides (membership in Type_list) is what keeps
std::vector<bool>::reference, user classes, char, long double, … out of the candidate set —
and it also makes type_promote_t well-formed, so the constraint is checked before any hard error
can escape; at least one operand must be complex; and !std_complex_handles_v leaves the pairs
std::complex already owns (complex<T> with complex<T> or with T) to std, so the cytnx
template can never become a second equally-good candidate. One operator== is enough — C++20
synthesizes scalar == complex from the reversed candidate and != from the negation.

There is still a breaking change, but it is a promotion change rather than a removal, and I
want it flagged explicitly. Results now fold through Type.type_promote, which crosses the
real/complex boundary by precision (#858, #982). The retired overloads returned complex64 for
complex64 op double — silently dropping the double's precision. That pair now yields
complex128. I believe that is the correct behavior and consistent with the rest of the dtype
work, but it is user-visible, so say the word if you would rather preserve the old return types.

The only outright removals are the UNI_GPU cuDoubleComplex/cuFloatComplex operator==
declarations (no call sites; #1019 owns the CUDA-side migration).

2. The requires branch in Kron — removed, you were right.

TO is type_promote_t<TL, TR> and to_complex is only reached when
is_complex(typeL) != is_complex(typeR), so the "two real inputs, complex output" case the Gemini
suggestion was optimizing for provably cannot arise. It is now simply:

out[i] = static_cast<TO>(Lin[x]) * static_cast<TO>(Rin[y]);

One tradeoff worth recording: this does cost a real multiply pair on complex × real, where the
native product is 2 real multiplies and the cast-both form is 4 + 2. I did not try to reclaim it —
it would need a much narrower guard than the one you rejected, and it is not what this PR is for.
Happy to open a follow-up if it is worth measuring.

3. developer_tools/Makefile — fixed.

Removed complex_arithmetic.o from OBJS and its build rule. Note the file is broadly stale
independently of this PR: 40+ of the paths it references no longer exist (Outer_internal.cpp,
Add_internal.cpp, the per-dtype *Storage.cpp, cucomplex_arithmetic.cu, …), from earlier
refactors. I kept the diff to the two lines this PR is responsible for rather than adopting that
cleanup here.

4. Rebased.

On current master. Outer_internal.cpp was indeed already deleted by #1105, so that half of the
patch disappeared; the diff is now the header rewrite, Kron_internal.hpp, Physics.cpp, the two
build-file lines, and tests.

5. Tests — the claims are committed now, not just asserted.

• tests/overload_hygiene_test.cpp: the vector<bool> guard, the concept's admit/reject set, and
  the mixed complex/scalar policy — result dtypes via static_assert plus exact values for
  + - * / == across negative, fractional, unsigned, bool and cross-precision operands.
• tests/Physics_test.cpp: spin-½ and spin-1 x/y/z against textbook ħ = 1 values, plus an
  independent algebraic check that [Sx, Sy] = i Sz for S = ½, 1, 3/2, 2.
• tests/linalg_test/Kron_test.cpp: ComplexDouble × ComplexFloat, ComplexFloat × Double,
  complex × signed integer, Uint32 × Int64, and a same-dtype control.

One correction to the earlier claim on this PR: the previous version of the hygiene guard did
not reproduce the bug. It used a const std::vector<bool>&, and the const operator[] returns
a plain bool rather than the proxy reference — so it compiled fine on pre-fix master. The
guard now takes a non-const vector (with a static_assert on the proxy type so it cannot decay
that way again) and does fail on pre-fix master: 5 ambiguous overload for 'operator==' errors
under GCC 13.3 / C++20.

Local gates on this rebase:

• debug-openblas-cpu (Debug + ASan): 1836/1836 ctest.
• pytest pytests/: 255 passed, 1 skipped.
• pycytnx builds and imports; cytnx.physics.spin(0.5, 'y') is correct.
• USE_CUDA=ON build of the library, test_main and gpu_test_main (sm_89, CUDA 13.0).
• pre-commit clean (clang-format v14).

On the GPU suite: two tests abort with CUSOLVER_STATUS_EXECUTION_FAILED from
cusolverDnDgesvdj at cuGeSvd_internal.cu:244 — GesvdTruncate.GpuFlagCombinations and
Rsvd.GpuFlagCombinations. Both reproduce identically on master (2318026), so they are
pre-existing and unrelated to this PR; I mention them only because the GPU suite is not in CI and
they will bite the next person who runs it locally. The remainder of the suite is still running as
I post this; I will follow up if anything else turns up.

---

Bot threads, for the record:

• Physics.cpp pow(x, 0.5) → std::sqrt (@gemini-code-assist): applied, and the redundant
  cytnx_double cast dropped.
• Kron_internal.hpp / Outer_internal.cpp cast-both-to-TO (@gemini-code-assist): not
  applied — see point 2. The if constexpr (requires …) form is the one Ian asked to be removed;
  the optimization it targets cannot occur given how TO is derived. Outer_internal.cpp no
  longer exists (deleted by refactor(linalg): implement Outer as Kron+reshape, retire Outer dispatch (#1003) #1105).
• "Preserve complex-scalar arithmetic" (@chatgpt-codex-connector): correct, and now resolved —
  the operators are preserved rather than deleted. See point 1.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[yingjerkao]

Follow-up on the GPU suite, as promised — it finished, and nothing in it is caused by this PR. Three tests fail, all reproduced on master (2318026) with the same build config:

test	cause	on master?
GesvdTruncate.GpuFlagCombinations	CUSOLVER_STATUS_EXECUTION_FAILED from cusolverDnDgesvdj, cuGeSvd_internal.cu:244	fails identically
Rsvd.GpuFlagCombinations	same cusolverDnDgesvdj call	fails identically
linalg_Test.GpuBkFUtQr	QR decomposition on GPU not implemented without cuQuantum support! (Qr.cpp:84) — this build is USE_CUQUANTUM=OFF	fails identically

Everything else passes (a handful of Rsvd/Svd/ExpM cases self-skip for the same cuQuantum reason).

The third one is arguably a test bug rather than an environment problem: linalg_Test.GpuBkFUtQr hard-fails without cuQuantum while its neighbours in Rsvd/RsvdNoTruncation GTEST_SKIP() on the same capability. Out of scope here, but happy to open an issue if that is worth tracking — the GPU suite is not in CI, so this only shows up when someone runs it locally.

Environment: RTX 4070 Ti SUPER (sm_89), CUDA 13.0, USE_CUDA=ON USE_CUTENSOR=OFF USE_CUQUANTUM=OFF.