Add reversible integer arithmetic primitives by wsttiger · Pull Request #47 · NVIDIA/cudaq-algorithms

wsttiger · 2026-09-08T17:09:42Z

Adds cudaq_algorithms.primitives reversible integer arithmetic device kernels — little-endian, in-place, with hand-written inverses (no cudaq.adjoint, per cuda-quantum#4897/#4898).

CDKM/Cuccaro ripple-carry (arXiv:quant-ph/0410184):

add_register / subtract_register — b ← (a ± b) mod 2ⁿ, 2n−2 Toffolis (the unused top carry-out cancels; §4.1's 2n−3 not adopted).
add_constant / subtract_constant — constant loaded into a caller-provided work+carry register, 2n−2.
cmp_ge_constant — out ^= (x ≥ K), restores x, 2n−1 Toffolis.
cmp_ge_register / cmp_gt_register — out ^= (a ≥ b) / (a > b) on equal-width unsigned registers, 2n−1, no n-qubit work register.

Draper QFT (arXiv:quant-ph/0008033, no Toffolis — r1 rotations only):

add_constant_qft / subtract_constant_qft.
cmp_ge_constant_qft_shift / cmp_ge_constant_qft_shift_adj — the compute half leaves x shifted by −K (for the Add alias sampling #48 alias sampler); the adjoint restores it.

Tests (92): operation-pinning harnesses — fixed-operand superposition for the register ops, basis-input truth tables for the constant ops, correlated-flag superposition for the comparators (over out ∈ {0,1} and K ∈ 0..2ⁿ) — op-then-inverse identity checks, and estimate_resources contracts against the documented per-kernel counts.

Follow-ups: ancilla-light constant adder (Häner, arXiv:1611.07995); Cuccaro §4.1's 2n−3 adder; cudaq.control composition once the compiler regression is fixed (issue forthcoming).

CDKM/Cuccaro ripple-carry family (register and constant add/subtract, >=-constant comparator) and the ancilla-free Draper QFT family (qft/iqft, Fourier-basis constant add/subtract, the comparator pair with its hand-written adjoint), all little-endian in-place kernels with hand-written inverses. Tests are exhaustive over all inputs for widths up to 5 via the superposition harness, pin every inverse by op-then-inverse identity, and hold the documented gate prices against the compiler with cudaq.estimate_resources: exactly 2n Toffolis for every CDKM operation (0 at K = 0), and zero Toffolis for the QFT family with exact controlled-r1 / r1 / h budgets (n(n-1) + n for the adder, n(n+1) + n + 1 per comparator side). Review fixes: document the 0 <= K <= 2^n precondition on cmp_ge_constant_qft (the borrow wraps mod 2^(n+1) for larger K) and the exact constant_bits/complement_bits length preconditions on add_constant and cmp_ge_constant; note in the module docstring that operands of any one op must be pairwise disjoint; version-scope the cudaq.adjoint limitation (broken as of CUDA-Q 0.15, still unresolved on the 0.16 pre-release) instead of stating it timelessly. Signed-off-by: Scott Thornton <wsttiger@gmail.com>

…amily cmp_ge_register(a, b, carry, out) XOR-loads (a >= b) (unsigned, equal-width little-endian registers) into the out qubit and restores a, b and the carry ancilla: complement b in place (2^n - b = ~b + 1, carry-in set to 1), MAJ sweep, copy the top ripple carry into out, reverse the sweep and undo the complement. Because b doubles as the constant-load register, no n-qubit work register is needed — only the one-qubit carry — and the price is the family contract of exactly 2n Toffolis, pinned against cudaq.estimate_resources with runtime-argument harnesses at widths 1-5 and 8. cmp_gt_register is the free strict variant (a > b <=> not (b >= a): one X plus the >= comparator with the roles swapped), same price. Both are self-inverse (compute-copy- uncompute operand action, XOR-accumulated flag), pinned by apply-twice-is-identity tests on random entangled states, alongside exhaustive truth tables (widths 1-4, flag initially 0 and 1) and superposed-register statevector checks against dense NumPy references. This is the comparator the alias-sampling conditional (keep-value vs alt-value test) and the first-quantized momentum comparisons need (colleague review feedback; GAP_first_quantized_lcu.md). Signed-off-by: Scott Thornton <wsttiger@gmail.com>

kvmto

Questions on the PR description:

add_constant and cmp_ge_constant are listed without work and carry. Which signature is right?
cmp_ge_register and cmp_gt_register are missing from the list. Can you add them?
It says 62 tests. The file collects 96. Which is it?
"Exhaustive truth tables for every operation." Which adder test is a truth table?
"Random superposed states" in the adjointness checks. Only the register comparators use random states. Can we reword?
The design note says these compose under cudaq.control. On 0.15.1 they do not. Can we remove or qualify this until there is a test?

Inline questions on the code and tests below.

kvmto · 2026-09-19T22:26:20Z

+"""Reversible in-place integer arithmetic device kernels.
+
+Two families, both little-endian (``register[0]`` is the least-significant
+bit, matching ``docs/conventions.md``):
+
+- **CDKM/Cuccaro ripple-carry** (`arXiv:quant-ph/0410184`):
+  ``add_register`` / ``subtract_register`` (in-place ``b <- b +/- a``),
+  ``add_constant`` / ``subtract_constant`` (constant loaded into a caller
+  provided work register), ``cmp_ge_constant`` (a ``x >= K`` comparator
+  writing into an out qubit, leaving ``x`` untouched: MAJ sweep, copy the
+  carry, reverse MAJ sweep), and the register-register comparators
+  ``cmp_ge_register`` / ``cmp_gt_register`` (``out ^= (a >= b)`` /
+  ``out ^= (a > b)`` on equal-width unsigned registers, same
+  MAJ-copy-reverse structure with ``b`` complemented in place, so no
+  ``n``-qubit work register at all — only the one-qubit carry). Toffoli
+  prices (documented contracts, pinned by the resource tests in
+  ``tests/python/test_primitives_arithmetic.py``):
+  ``add_register`` / ``subtract_register`` cost exactly ``2 n`` Toffolis
+  on ``n``-bit registers (``n`` in the MAJ sweep, ``n`` in the UMA sweep),
+  ``add_constant`` / ``subtract_constant`` inherit the same ``2 n`` (the
+  constant load is X-only), ``cmp_ge_constant`` costs ``2 n`` for
+  ``K >= 1`` (MAJ sweep plus its reversal) and ``0`` for ``K = 0``, and
+  ``cmp_ge_register`` / ``cmp_gt_register`` cost exactly ``2 n`` (the
+  complement and flag copy are X/CNOT-only).
+- **Draper QFT arithmetic** (`arXiv:quant-ph/0008033`): ``qft`` / ``iqft``
+  and ``add_constant_qft``, plus the ``cmp_ge_constant_qft`` /
+  ``cmp_ge_constant_qft_adj`` comparator pair. The QFT family needs *no*
+  work qubits — on a statevector simulator every extra work qubit doubles
+  the memory, and the CDKM constant ops need ``n + 1`` of them — and *no*
+  Toffolis: its price is rotations, ``n (n - 1)`` controlled-``r1`` plus
+  ``n`` free ``r1`` for ``add_constant_qft`` on ``n`` bits, and
+  ``n (n + 1)`` controlled-``r1`` plus ``n + 1`` free ``r1`` per side of
+  the ``cmp_ge_constant_qft`` pair (``K >= 1``; zero rotations at
+  ``K = 0``), also pinned by the resource tests.
+
+Registers passed to any one op must be pairwise disjoint: the kernels
+never alias-check, and overlapping views (e.g. ``add_register(a, a,
+carry)``, or a ``work`` view sharing qubits with ``target``) are
+undefined.
+
+Every inverse is hand-written (``cudaq.adjoint`` is off-limits as of
+CUDA-Q 0.15: cuda-quantum#4897/#4898, still unresolved as of the 0.16
+pre-release) and pinned by op-then-inverse identity tests in
+``tests/python/test_primitives_arithmetic.py``, alongside exhaustive
+value checks against classical integer arithmetic for widths up to 5.
+
+Kernel-language notes (see CLAUDE.md): guards are positive ``if`` blocks
+(kernel ``return`` is silently ignored, cuda-quantum#4845), and all
+classical indices are recomputed from loop variables rather than carried
+as mutating accumulators.
+"""


Can we cut this docstring to a few lines plus the two arXiv links, and move each gate price to the kernel that owns it? Same house style as PR #40.

kvmto · 2026-09-19T22:26:20Z

+Every inverse is hand-written (``cudaq.adjoint`` is off-limits as of
+CUDA-Q 0.15: cuda-quantum#4897/#4898, still unresolved as of the 0.16
+pre-release) and pinned by op-then-inverse identity tests in
+``tests/python/test_primitives_arithmetic.py``, alongside exhaustive


We distrust cudaq.adjoint here. Why do we trust cudaq.control without a test?

kvmto · 2026-09-19T22:26:20Z

+        # UMA sweep (reverse).
+        for k in range(1, n):
+            i = n - k
+            x.ctrl(a[i - 1], b[i], a[i])


This Toffoli and the one at L104 are the same gate on the same qubits with nothing in between. Don't they cancel?

After removing them, the two cx(a[i], a[i-1]) at the top bit also cancel. Is the count then 2n-2, not 2n?

kvmto · 2026-09-19T22:26:20Z

+
+
+@cudaq.kernel
+def add_register(a: cudaq.qview, b: cudaq.qview, carry: cudaq.qview):


For n = 1 this emits two Toffolis. Isn't a 1-bit mod-2 adder a single CNOT?

kvmto · 2026-09-19T22:26:20Z

+  prices (documented contracts, pinned by the resource tests in
+  ``tests/python/test_primitives_arithmetic.py``):
+  ``add_register`` / ``subtract_register`` cost exactly ``2 n`` Toffolis
+  on ``n``-bit registers (``n`` in the MAJ sweep, ``n`` in the UMA sweep),


Cuccaro Table 1 gives 2n-3 Toffolis for a mod 2^n adder. Why do we pin 2n as the contract?

kvmto · 2026-09-19T22:26:20Z

+
+@pytest.mark.parametrize("n", [1, 2, 3, 4])
+@pytest.mark.parametrize("name,strict,predicate", _CMP_REGISTER_OPS)
+def test_cmp_register_truth_table_exhaustive(n, name, strict, predicate):


These are the strongest tests in the file. Can the adder tests follow this pattern?

kvmto · 2026-09-19T22:26:21Z

+def test_add_register_basis_spot_checks():
+    for n, aval, bval in [(2, 1, 2), (3, 5, 6), (5, 21, 27)]:
+        state = np.array(cudaq.get_state(_spot_add, n, aval, bval))
+        index = aval + (((aval + bval) % (1 << n)) << n)
+        np.testing.assert_allclose(state, _basis(index, 2 * n + 1), atol=1e-12)


Only three spot checks and only for add_register. Why not a full truth table at small n?

kvmto · 2026-09-19T22:26:21Z

+
+@_RESOURCES
+@pytest.mark.parametrize("n", WIDTHS)
+def test_cdkm_register_ops_cost_exactly_2n_toffolis(n):


If the adder becomes 2n-2 or 2n-3, these all change. Should the contract reference Cuccaro Table 1 rather than a derived number?

kvmto · 2026-09-19T22:26:21Z

+def test_cdkm_toffoli_cost_grows_linearly_per_doubling():
+    # The cost is exactly linear (2 n): each width doubling doubles it.
+    compiled = {n: _toffolis(_res_add_register, n, 0) for n in (2, 4, 8, 16)}
+    for n in (2, 4, 8):
+        assert compiled[2 * n] == 2 * compiled[n], compiled
+
+


The doubling law only holds for exactly 2n. Is this test asserting the optimisation is absent?

kvmto · 2026-09-19T22:26:21Z

+
+from cudaq_algorithms.primitives import _arithmetic as arith
+
+WIDTHS = [1, 2, 3, 4, 5]


Where is the cudaq.control test? On 0.15.1, direct control of seven of these kernels fails to compile, and control through a one-line wrapper compiles but fires on both branches. Have you seen this? Should we file it upstream?

Sam asked in PR #40 for tests with a superposed control. Do we have one here?

kvmto

Questions on the PR description:

add_constant and cmp_ge_constant are listed without work and carry. Which signature is right? The code has both, so the description looks stale.
cmp_ge_register and cmp_gt_register are missing from the list. Can you add them?
It says 62 tests. The file collects 96. Which is it?
"Exhaustive truth tables for every operation." Which adder test is a truth table? Only the register comparators and the three spot checks are.
"Random superposed states" in the adjointness checks. Only the register comparators use random states. Can we reword?
The design note says these compose under cudaq.control. On 0.15.1 they do not. Can we remove or qualify this until there is a test?

Inline questions on the code and tests below.

kvmto · 2026-09-19T22:37:37Z

+"""Reversible in-place integer arithmetic device kernels.
+
+Two families, both little-endian (``register[0]`` is the least-significant
+bit, matching ``docs/conventions.md``):
+
+- **CDKM/Cuccaro ripple-carry** (`arXiv:quant-ph/0410184`):
+  ``add_register`` / ``subtract_register`` (in-place ``b <- b +/- a``),
+  ``add_constant`` / ``subtract_constant`` (constant loaded into a caller
+  provided work register), ``cmp_ge_constant`` (a ``x >= K`` comparator
+  writing into an out qubit, leaving ``x`` untouched: MAJ sweep, copy the
+  carry, reverse MAJ sweep), and the register-register comparators
+  ``cmp_ge_register`` / ``cmp_gt_register`` (``out ^= (a >= b)`` /
+  ``out ^= (a > b)`` on equal-width unsigned registers, same
+  MAJ-copy-reverse structure with ``b`` complemented in place, so no
+  ``n``-qubit work register at all — only the one-qubit carry). Toffoli
+  prices (documented contracts, pinned by the resource tests in
+  ``tests/python/test_primitives_arithmetic.py``):
+  ``add_register`` / ``subtract_register`` cost exactly ``2 n`` Toffolis
+  on ``n``-bit registers (``n`` in the MAJ sweep, ``n`` in the UMA sweep),
+  ``add_constant`` / ``subtract_constant`` inherit the same ``2 n`` (the
+  constant load is X-only), ``cmp_ge_constant`` costs ``2 n`` for
+  ``K >= 1`` (MAJ sweep plus its reversal) and ``0`` for ``K = 0``, and
+  ``cmp_ge_register`` / ``cmp_gt_register`` cost exactly ``2 n`` (the
+  complement and flag copy are X/CNOT-only).
+- **Draper QFT arithmetic** (`arXiv:quant-ph/0008033`): ``qft`` / ``iqft``
+  and ``add_constant_qft``, plus the ``cmp_ge_constant_qft`` /
+  ``cmp_ge_constant_qft_adj`` comparator pair. The QFT family needs *no*
+  work qubits — on a statevector simulator every extra work qubit doubles
+  the memory, and the CDKM constant ops need ``n + 1`` of them — and *no*
+  Toffolis: its price is rotations, ``n (n - 1)`` controlled-``r1`` plus
+  ``n`` free ``r1`` for ``add_constant_qft`` on ``n`` bits, and
+  ``n (n + 1)`` controlled-``r1`` plus ``n + 1`` free ``r1`` per side of
+  the ``cmp_ge_constant_qft`` pair (``K >= 1``; zero rotations at
+  ``K = 0``), also pinned by the resource tests.
+
+Registers passed to any one op must be pairwise disjoint: the kernels
+never alias-check, and overlapping views (e.g. ``add_register(a, a,
+carry)``, or a ``work`` view sharing qubits with ``target``) are
+undefined.
+
+Every inverse is hand-written (``cudaq.adjoint`` is off-limits as of
+CUDA-Q 0.15: cuda-quantum#4897/#4898, still unresolved as of the 0.16
+pre-release) and pinned by op-then-inverse identity tests in
+``tests/python/test_primitives_arithmetic.py``, alongside exhaustive
+value checks against classical integer arithmetic for widths up to 5.
+
+Kernel-language notes (see CLAUDE.md): guards are positive ``if`` blocks
+(kernel ``return`` is silently ignored, cuda-quantum#4845), and all
+classical indices are recomputed from loop variables rather than carried
+as mutating accumulators.
+"""


Can we cut this docstring to a few lines plus the two arXiv links, and move each gate price to the kernel that owns it? Same house style as PR #40. Keep the little-endian convention, the disjointness rule, and the cuda-quantum issue link for hand-written inverses. Everything else belongs on the kernel or in the paper.

kvmto · 2026-09-19T22:37:37Z

+Every inverse is hand-written (``cudaq.adjoint`` is off-limits as of
+CUDA-Q 0.15: cuda-quantum#4897/#4898, still unresolved as of the 0.16
+pre-release) and pinned by op-then-inverse identity tests in
+``tests/python/test_primitives_arithmetic.py``, alongside exhaustive


We distrust cudaq.adjoint here. Why do we trust cudaq.control without a test? On CUDA-Q 0.15.1, direct control of add_constant, subtract_constant, cmp_gt_register and the four QFT wrappers fails with "Could not successfully apply kernel specialization". Controlling a one-line wrapper kernel compiles but applies the op on both control branches. 0.14.2 is correct. Worth a numerical test with a superposed control, and an upstream issue.

kvmto · 2026-09-19T22:37:37Z

+        # UMA sweep (reverse).
+        for k in range(1, n):
+            i = n - k
+            x.ctrl(a[i - 1], b[i], a[i])


This Toffoli and the one at L104 are the same gate on the same qubits with nothing in between. Don't they cancel? At i = n-1 the MAJ Toffoli computes the carry-out c_n into a[n-1] and the first UMA Toffoli uncomputes it. A mod 2^n adder never reads c_n. After removing them, the two cx(a[n-1], a[n-2]) also cancel. What survives at the top bit is cx(a[n-1], b[n-1]) then cx(a[n-2], b[n-1]). I checked this agrees on all basis inputs for n = 2, 3, 4 with 2n-2 Toffolis.

kvmto · 2026-09-19T22:37:37Z

+
+
+@cudaq.kernel
+def add_register(a: cudaq.qview, b: cudaq.qview, carry: cudaq.qview):


For n = 1 this emits two Toffolis. Isn't a 1-bit mod-2 adder a single CNOT? Same cancellation as at L104/L108, degenerate case.

kvmto · 2026-09-19T22:37:37Z

+  prices (documented contracts, pinned by the resource tests in
+  ``tests/python/test_primitives_arithmetic.py``):
+  ``add_register`` / ``subtract_register`` cost exactly ``2 n`` Toffolis
+  on ``n``-bit registers (``n`` in the MAJ sweep, ``n`` in the UMA sweep),


Cuccaro Table 1 gives 2n-3 Toffolis for a mod 2^n adder (n >= 3). Why do we pin 2n as the contract? Section 4.1 of the paper runs an (n-1)-bit adder and finishes with one CNOT from a[n-1] to b[n-1]. Either adopt it, or pin 2n-2 from the cancellation and say why we stop there.

kvmto · 2026-09-19T22:37:38Z

+
+@pytest.mark.parametrize("n", [1, 2, 3, 4])
+@pytest.mark.parametrize("name,strict,predicate", _CMP_REGISTER_OPS)
+def test_cmp_register_truth_table_exhaustive(n, name, strict, predicate):


These are the strongest tests in the file. Can the adder tests follow this pattern?

kvmto · 2026-09-19T22:37:38Z

+def test_add_register_basis_spot_checks():
+    for n, aval, bval in [(2, 1, 2), (3, 5, 6), (5, 21, 27)]:
+        state = np.array(cudaq.get_state(_spot_add, n, aval, bval))
+        index = aval + (((aval + bval) % (1 << n)) << n)
+        np.testing.assert_allclose(state, _basis(index, 2 * n + 1), atol=1e-12)


Only three spot checks and only for add_register. Why not a full truth table at small n? This is the only test that currently distinguishes add from subtract.

kvmto · 2026-09-19T22:37:38Z

+
+@_RESOURCES
+@pytest.mark.parametrize("n", WIDTHS)
+def test_cdkm_register_ops_cost_exactly_2n_toffolis(n):


If the adder becomes 2n-2 or 2n-3, these all change. Should the contract reference Cuccaro Table 1 rather than a derived number? State the count once per kernel docstring and have the test read that number.

kvmto · 2026-09-19T22:37:38Z

+def test_cdkm_toffoli_cost_grows_linearly_per_doubling():
+    # The cost is exactly linear (2 n): each width doubling doubles it.
+    compiled = {n: _toffolis(_res_add_register, n, 0) for n in (2, 4, 8, 16)}
+    for n in (2, 4, 8):
+        assert compiled[2 * n] == 2 * compiled[n], compiled
+
+


The doubling law only holds for exactly 2n. Is this test asserting the optimisation is absent? 2(2n) - 2 != 2(2n - 2). A direct per-width assertion would be more robust.

kvmto · 2026-09-19T22:37:38Z

+
+from cudaq_algorithms.primitives import _arithmetic as arith
+
+WIDTHS = [1, 2, 3, 4, 5]


Where is the cudaq.control test? Sam asked in PR #40 for a superposed control against an analytic reference. That test would have caught the 0.15.1 control drop described in the _arithmetic.py comment at L43-46.

…IDIA#47) Addresses kvmto's review. Tests -- the previous uniform-superposition harness was invariant under the operation (any bijection maps a uniform input to a uniform output), so add, subtract and identity all passed. Replaced with operation-pinning harnesses: register add/subtract fix one operand and superpose the other; the constant ops use basis-input truth tables; the comparators verify the correlated flag, now also over out in {0, 1} and K across 0 .. 2^n inclusive. Toffoli counts -- the mod-2^n adders never read the top carry-out, so its MAJ/UMA Toffoli pair (and the surrounding CX pair) cancels: add_register, subtract_register and the constant adders are now 2n - 2. The CDKM comparators write the top carry straight into out instead of compute/copy/uncompute, so cmp_ge_constant, cmp_ge_register and cmp_gt_register are 2n - 1. Contracts cite Cuccaro et al. (arXiv:quant-ph/0410184) Table 1; the doubling-law test now checks the affine slope. phase_add_constant reduces K mod 2^(t+1) as an integer before the rotation angle, so it is exact for any K (previously K was cast to float). Renamed cmp_ge_constant_qft -> cmp_ge_constant_qft_shift (and its adjoint) to make explicit that it leaves x_reg shifted to (x - K) mod 2^n; the docstring leads with that contract. Trimmed the module docstring and moved per-operation gate prices onto each kernel. Full arithmetic suite green.

…IDIA#47) Addresses kvmto's review. Tests -- the previous uniform-superposition harness was invariant under the operation (any bijection maps a uniform input to a uniform output), so add, subtract and identity all passed. Replaced with operation-pinning harnesses: register add/subtract fix one operand and superpose the other; the constant ops use basis-input truth tables; the comparators verify the correlated flag, now also over out in {0, 1} and K across 0 .. 2^n inclusive. Toffoli counts -- the mod-2^n adders never read the top carry-out, so its MAJ/UMA Toffoli pair (and the surrounding CX pair) cancels: add_register, subtract_register and the constant adders are now 2n - 2. (Cuccaro et al., arXiv:quant-ph/0410184, Section 4.1 reaches 2n - 3 via an (n-1)-bit adder plus a CNOT; not adopted, to keep the plain MAJ/UMA structure.) The CDKM comparators write the top carry straight into out instead of compute/copy/uncompute, so cmp_ge_constant, cmp_ge_register and cmp_gt_register are 2n - 1 (Cuccaro's comparator count). The doubling-law test now checks the affine slope. phase_add_constant reduces K mod 2^(t+1) as an integer before the rotation angle, so it is exact for any K (previously K was cast to float). Renamed cmp_ge_constant_qft -> cmp_ge_constant_qft_shift (and its adjoint) to make explicit that it leaves x_reg shifted to (x - K) mod 2^n; the docstring leads with that contract. Trimmed the module docstring and moved per-operation gate prices onto each kernel. Full arithmetic suite green.

wsttiger · 2026-09-20T16:19:49Z

Thanks @kvmto — this was a genuinely useful review; it caught real problems, especially the tests. Changes pushed in 6a93217; point-by-point below (grouped, since several threads are the same issue).

PR description

Rewritten to match the code. The stale signatures (add_constant/cmp_ge_constant do take work+carry), the missing cmp_ge_register/cmp_gt_register, and the "62 tests" (it collects 92 now) are all fixed. The "exhaustive truth tables for every operation" and "random superposed states" overclaims are gone.

Tests — the vacuity (L11/L77/L108, and L174 for the constant ops)

You're right, and thank you: the collapsed uniform-superposition check is invariant under the operation — any bijection sends a uniform input to the same uniform output, so add, subtract and identity all passed. Fixed:

register add/subtract now fix b and superpose a (looping all b): for a fixed b, a→a+b and a→b−a are different permutations, so the statevector distinguishes them, and it fails if the ops are swapped.
constant ops now use a basis-input truth table (a single register has no operand to hold fixed, so a definite input is needed to see K).
comparators: added out ∈ {0,1} (the XOR contract, L273) and K swept over 0 .. 2^n inclusive — both the K=0 and K=2^n boundaries (L175).

Toffoli counts (L83 / L211 / L279 / L513 / L551)

Adders: applied the cancellation you described — the mod-2ⁿ adder never reads the top carry-out, so its MAJ/UMA Toffoli pair and the surrounding CX pair cancel, leaving cx(a[n-1],b[n-1]); cx(a[n-2],b[n-1]). add_register/subtract_register (and the constant adders) are now 2n−2 (n=1→0), verified inverse-preserving by the strengthened tests. On L21: you're right that Cuccaro §4.1 reaches 2n−3 via an (n−1)-bit adder + final CNOT — I stopped at 2n−2 (the local cancellation) to keep the plain MAJ/UMA structure and said so in the docstring. Happy to adopt the 2n−3 construction if you'd prefer it.
Comparators: applied the one-Toffoli saving — write the top carry straight into out (CX(w;f) CCX(u,v;f)), leaving work[n-1] untouched. cmp_ge_constant and cmp_ge_register/cmp_gt_register are now 2n−1 (Cuccaro's comparator count).
Counts now live on each kernel docstring and the tests read that number; the doubling-law test asserts the affine slope (2n−2 doesn't double).

Overflow (L324)

Fixed. phase_add_constant reduces K mod 2^(t+1) as an integer per bit before the rotation angle, so it stays in [0,2π) and is exact for any K, including K ≥ 2⁵³.

Naming / shift (L380, L166, L377)

Renamed cmp_ge_constant_qft → cmp_ge_constant_qft_shift (+ _shift_adj); the docstring now leads with "leaves x_reg shifted to (x−K) mod 2^n" and contrasts it with cmp_ge_constant (x-preserving, out XOR-loaded, complement bits). The consumer that needs the shift is the alias sampler in #48.

`cudaq.control` (L46 / test L30)

Right to distrust it — removed the "composes under cudaq.control" claim. On 0.15.1 direct control of several of these fails specialization and the wrapper fires on both branches; I'll file the upstream issue with a minimal reproducer and add a skip-gated superposed-control test that un-skips once it's fixed.

Smaller

L162 add_constant is the straightforward CDKM-with-loaded-constant, not a placeholder; noted Häner (arXiv:1611.07995) as the ancilla-light follow-up on the kernel.
L307 cmp_gt_register kept as a named kernel for a discoverable API, with the docstring stating it's cmp_ge_register with the operands swapped + the strict X.
L17 module docstring trimmed to the conventions + arXiv links; per-gate prices moved onto the kernels.

Full arithmetic suite green (92).

cudaq.control of a device kernel that calls a sub-kernel regressed on CUDA-Q 0.15.x (kernel-specialization failure); it is correct on 0.14.2 and again on 0.16.0. Since the library targets 0.16, pin the controlled-composition behaviour with a superposed-control numerical check on a composed kernel (controlled add_constant_qft), skipped on the affected 0.15.x line. The superposed control also catches a control that fires on both branches.

wsttiger · 2026-09-20T20:33:26Z

Follow-up on the cudaq.control thread (L27/L46): I bisected it.

cudaq.control of a device kernel that calls a sub-kernel fails on 0.15.1 with the specialization error, but is correct on 0.14.2 (before) and again on 0.16.0 (after) — a 0.15.x-only regression, already fixed upstream. Flat kernels (no sub-kernel call) control fine even on 0.15.1; the failing cases are exactly the ones that call sub-kernels (add_constant, subtract_constant, cmp_gt_register, the QFT wrappers).

Since the library targets 0.16, controlled composition genuinely works — so rather than file an already-fixed bug, I added a version-gated superposed-control test (d9bc49a): it controls a composed kernel (add_constant_qft) with the control in superposition and checks the result numerically, skipping below 0.16. A superposed control also catches a control that fires on both branches, so it doubles as the "is it really controlled?" check.

(Aside: on 0.15.1 I couldn't reproduce the silent "fires on both branches" you saw — here the composed case hard-fails specialization rather than mis-controlling. Either way it's gone in 0.16, so it's moot for the library.)

wsttiger requested a review from kvmto September 11, 2026 16:11

wsttiger added 2 commits September 15, 2026 16:21

wsttiger force-pushed the features/primitives_arithmetic branch from 83f0264 to a412e3b Compare September 15, 2026 16:22

wsttiger mentioned this pull request Sep 15, 2026

Rebuild select_swap as composed kernels; promise the composition contract (cudaq >= 0.16) #52

Open

wsttiger added the enhancement New feature or request label Sep 17, 2026

kvmto reviewed Sep 19, 2026

View reviewed changes

wsttiger force-pushed the features/primitives_arithmetic branch from d7b3999 to 6a93217 Compare September 20, 2026 16:19



		@cudaq.kernel
		def add_register(a: cudaq.qview, b: cudaq.qview, carry: cudaq.qview):


		from cudaq_algorithms.primitives import _arithmetic as arith

		WIDTHS = [1, 2, 3, 4, 5]

Conversation

wsttiger commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kvmto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kvmto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wsttiger commented Sep 20, 2026

PR description

Tests — the vacuity (L11/L77/L108, and L174 for the constant ops)

Toffoli counts (L83 / L211 / L279 / L513 / L551)

Overflow (L324)

Naming / shift (L380, L166, L377)

cudaq.control (L46 / test L30)

Smaller

Uh oh!

wsttiger commented Sep 20, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wsttiger commented Sep 8, 2026 •

edited

Loading

`cudaq.control` (L46 / test L30)