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

[yingjerkao]

Problem

Ian's fold-in from the #1003 review. include/utils/complex_arithmetic.hpp declared ~300 namespace-scope operators (+ - * / ==) between cytnx_complex64/128 and every builtin scalar. Because cytnx_complex64/128 are aliases for std::complex<float/double> and builtins convert into them, these leak into ordinary overload resolution for unrelated code — amplified by C++20's reversed operator== rewrite. The reported symptom:

using namespace cytnx;
std::vector<bool> v(1); bool b;
v[0] == b;   // error: ambiguous operator== (reversed cytnx complex candidates vs builtin)

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

Delete complex_arithmetic.{hpp,cpp} and its three include sites. std::complex already provides complex<>–real arithmetic and comparison; the only load-bearing cases were mixed complex128 × complex64 (which std::complex doesn't define). A CPU build with the header removed pinned the entire blast radius to three files:

• Kron_internal.hpp / Outer_internal.cpp — compute static_cast<TO>(a) * static_cast<TO>(b) (the output storage type), matching the Merge CUDA unary and binary elementwise operations through one typed kernel framework #1003 "compute through the operation's output type" rule.
• Physics.cpp (spin 'y') — build the coefficient as a real and construct the imaginary entry directly instead of complex(0,1) * real.

The == overloads had zero call sites — pure pollution.

Adds tests/overload_hygiene_test.cpp: a compile-time guard (with runtime asserts) for the vector<bool>-reference-vs-bool case; it stops compiling if the operators return.

Testing

CPU (openblas): library + test_main build clean; OverloadHygiene passes (and fails to compile on pre-fix master — reproduced). An independent numeric harness confirms Kron/Outer of ComplexDouble × ComplexFloat promote to ComplexDouble with exact mixed products, and spin(½,'y') == [[0,-i/2],[i/2,0]]. GPU-config compile-check of utils_internal_interface.cpp and cuOuter_internal.cu passes (the deleted header's UNI_GPU cuComplex== block had no users).

Note

Marked ! because it removes public namespace-scope operators. In practice std::complex covers complex<>–real use; only mixed complex128 × complex64 (rare in downstream code) loses an implicit operator. Independent of the GPU-side #1003 work in the sibling branch.

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.

[gemini-code-assist]

Casting both operands to the output type TO before multiplying them can be highly inefficient. For example, if both inputs are real (double) but the output is complex (cytnx_complex128), casting them first forces a complex multiplication (which requires 4 real multiplications and 2 additions) instead of a single real multiplication. Since Cytnx targets C++20, we can use if constexpr with a requires expression to check if the operands can be multiplied directly, falling back to casting only when mixed complex types are involved.

        if constexpr (requires(TL l, TR r) { l * r; }) {
          out[i] = static_cast<TO>(Lin[x] * Rin[y]);
        } else {
          out[i] = static_cast<TO>(Lin[x]) * static_cast<TO>(Rin[y]);
        }

[gemini-code-assist]

Casting both operands to the output type TO before multiplying them can be highly inefficient. For example, if both inputs are real but the output is complex, casting them first forces a complex multiplication instead of a single real multiplication. Since Cytnx targets C++20, we can use if constexpr with a requires expression to check if the operands can be multiplied directly, falling back to casting only when mixed complex types are involved.

        if constexpr (requires(T1 l, T2 r) { l * r; }) {
          _out[r] = static_cast<TO>(_Lin[cytnx_uint64(r / j2)] * _Rin[(r % j2)]);
        } else {
          _out[r] = static_cast<TO>(_Lin[cytnx_uint64(r / j2)]) * static_cast<TO>(_Rin[(r % j2)]);
        }

[gemini-code-assist]

Using pow(..., 0.5) is less efficient and less readable than std::sqrt. Additionally, the explicit cast to cytnx_double is redundant because pow (and std::sqrt) already returns a double. According to the repository style guide, we should avoid propagating legacy cytnx_XXXX typedefs for local arithmetic types and prefer standard C++ types like double.

Suggested change

	Out.at<cytnx_complex128>({a, a - 1}) =
	cytnx_complex128(0, 1) * pow((S + 1) * (2 * a) - (a + 1) * a, 0.5) / 2;
	cytnx_complex128(0, cytnx_double(pow((S + 1) * (2 * a) - (a + 1) * a, 0.5)) / 2);
	if (a != Dim - 1)
	Out.at<cytnx_complex128>({a, a + 1}) =
	cytnx_complex128(0, -1) * pow((S + 1) * (2 * a + 2) - (a + 2) * (a + 1), 0.5) / 2;
	Out.at<cytnx_complex128>({a, a + 1}) = cytnx_complex128(
	0, -(cytnx_double(pow((S + 1) * (2 * a + 2) - (a + 2) * (a + 1), 0.5)) / 2));
	Out.at<cytnx_complex128>({a, a - 1}) =
	cytnx_complex128(0, std::sqrt((S + 1) * (2 * a) - (a + 1) * a) / 2);
	if (a != Dim - 1)
	Out.at<cytnx_complex128>({a, a + 1}) =
	cytnx_complex128(0, -std::sqrt((S + 1) * (2 * a + 2) - (a + 2) * (a + 1)) / 2);

References

1. For new or touched code, avoid propagating legacy cytnx_XXXX typedefs as ordinary loop counters, sizes, flags, or local arithmetic types. Prefer standard C++ types unless the value is specifically a dtype-backed tensor/storage scalar. ^(link)

[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 73.67%. Comparing base (98de76f) to head (41070f4).
⚠️ Report is 111 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #1092       +/-   ##
===========================================
+ Coverage   57.98%   73.67%   +15.69%     
===========================================
  Files         229      225        -4     
  Lines       33459    27771     -5688     
  Branches       71       71               
===========================================
+ Hits        19401    20461     +1060     
+ Misses      14036     7289     -6747     
+ Partials       22       21        -1     

Flag	Coverage Δ
cpp	73.81% <100.00%> (+15.89%)	⬆️
python	64.13% <ø> (+0.52%)	⬆️

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

Components	Coverage Δ
C++ backend	73.08% <100.00%> (+17.29%)	⬆️
Python bindings	77.70% <ø> (+5.68%)	⬆️
Python package	64.13% <ø> (+0.52%)	⬆️

Files with missing lines	Coverage Δ
src/Physics.cpp	28.03% <100.00%> (ø)
src/backend/linalg_internal_cpu/Kron_internal.hpp	100.00% <100.00%> (ø)
src/backend/linalg_internal_cpu/Outer_internal.cpp	3.22% <100.00%> (ø)

... and 52 files 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 98de76f...41070f4. 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".

[chatgpt-codex-connector]

Preserve complex-scalar arithmetic

With this umbrella include removed, C++ callers that include cytnx.hpp/utils.hpp and use the public cytnx_complex64/128 aliases with integer or mismatched-precision scalars lose previously available arithmetic/comparison: for example cytnx_complex128 z; z + cytnx_int32{1} and z == 0 are not provided by std::complex because its non-member templates require the scalar to deduce as the same T. The change note says only mixed complex128 × complex64 loses support, but the deleted overloads also covered these common complex/builtin cases, so this is a broader source break for downstream C++ users.

Useful? React with 👍 / 👎.

[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.