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 ← (b ± a) 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. out is XOR-loaded, with the argument order matching the CDKM comparators (out last).

Tests (100 passing; 2 version-gated skips): 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ⁿ, for both comparator families) — op-then-inverse identity checks, direct qft/iqft and large-K coverage, a public-API resolution check, and estimate_resources contracts against the documented per-kernel counts.

cudaq.control: composition over these kernels was a 0.15.x specialization regression — correct on 0.14.2 and fixed on 0.16.0 (no upstream issue filed). A version-gated superposed-control test pins it on 0.16 and skips on 0.15.x.

Follow-ups: ancilla-light constant adder (Häner, arXiv:1611.07995); Cuccaro §4.1's 2n−3 adder.

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

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.

…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.)

kvmto

Questions on the PR description:

Line 4 writes b <- (a ± b) mod 2^n. For the minus sign that reads a - b; the kernel computes b - a. Suggest b <- (b ± a).
"92 tests" should be 94. "K ∈ 0..2^n" holds only for the CDKM comparator.
"cudaq.control composition once the compiler regression is fixed (issue forthcoming)" and the thread reply "no upstream issue, fixed in 0.16" still disagree. Which is it?

Formatting: yapf 0.43.0 over the two files (clears the red CI check). Correctness / robustness: - cmp_ge_constant_qft_shift: reduce K mod 2^(t+1) as an integer before the rotation (as phase_add_constant does), so it is exact on fp32 targets and for large K, not only on fp64 qpp-cpu. - cmp_gt_register: own n=1 branch (a AND ~b) instead of deferring to cmp_ge_register, so the two cancelling X gates on out are never emitted. - add_register / subtract_register: drop the UMA carry-in CNOT that is the identity under the carry=|0> precondition (no test could pin it). - cmp_ge_constant: use elif n > 1 (like its siblings) so n = 0 is a no-op rather than an index error. API / docs: - cmp_ge_constant_qft_shift(_adj): move out to the last argument, matching the CDKM comparators. Document that out is XOR-loaded (not |0>-only), that K <= 0 leaves x unshifted, the n <= 61 width limit, and the cheaper restore via add_constant_qft. Fix the stale CDKM carry-out comment and the cmp_ge_constant cross-reference. State the per-kernel adjoint policy (CDKM needs it; Draper does not) and the first-operand-width / aliasing (0.16 hang) caveats. - add_constant: correct the constant_bits length wording (longer is K mod 2^n; shorter reads past the host list). cmp_ge_constant: document the work/carry |0> precondition. phase_add_constant: document the n <= 62 int64 limit. - primitives/__init__: list phase_add_constant and note autodoc skips kernels. Tests: - qft/iqft tested directly (catches a composite-cancelling S mutant); large-K QFT adder test pins the integer reduction; cmp_ge_constant_qft_shift covers K = 2^n and an XOR-loaded out; public-package name-resolution test; QFT comparator adjoint rotation counts pinned; version parser now fails loudly instead of skipping the controlled test silently; stale 2n derivation block updated to 2n-2 / 2n-1; drop the n=4 register truth table (slow on 0.15.1; covered by the superposed test); drop unused parametrize args. Suite: 100 passed, 2 skipped on 0.15.1 (the 2 control-test cases run on 0.16).

wsttiger · 2026-10-04T17:15:55Z

Thanks for the thorough second pass -- this caught real issues, including two places my earlier replies overstated (the work/carry precondition and the K = 2^n shift-comparator coverage). All 27 are addressed in b62e5de: yapf is green, the QFT comparator now reduces K as an integer (fp32-exact) and takes out last, cmp_gt_register has a clean n = 1, and the test gaps you found (direct qft/iqft, large K, K = 2^n, XOR-loaded out, public name resolution, adjoint cost, loud version parse) are filled. Suite: 100 passed / 2 skipped on 0.15.1 (the 2 control cases run on 0.16).

wsttiger · 2026-10-06T16:54:22Z

@kvmto thanks — fixed the PR description:

b ← (a ± b) → b ← (b ± a) (the minus now reads b − a, matching subtract_register's b <- (b - a) mod 2^n).
Test count corrected to 100 passing + 2 version-gated skips (the stale "92" is gone).
K ∈ 0..2ⁿ now holds for both comparator families: as of b62e5de the QFT shift-comparator test sweeps K over 0..2^n inclusive (previously only the CDKM comparator did), so the description's claim is now accurate — I noted "both comparator families" explicitly.
The cudaq.control contradiction is resolved to match the thread: it was a 0.15.x specialization regression, correct on 0.14.2 and fixed on 0.16.0, with no upstream issue filed. The "(issue forthcoming)" line is removed; the description now states that and the version-gated test that pins it.

kvmto

Thanks, Scott. I rechecked b62e5de, including the latest clarifications and the CUDA-Q 0.16 controlled-composition behavior. I found no remaining blocking correctness concerns.
The additional test-coverage and API-documentation improvements can be handled as follow-ups. Approving.

Thanks!! Good job!!

PR NVIDIA#47 merged with the one-Toffoli comparator saving (cmp_ge_register is now 2n-1, not 2n). The alias-sampling PREPARE consumes that comparator, so its documented Toffoli cost drops by one: qrom.toffoli_count + 2*mu - 1 (was + 2*mu). Updates the sampler docstring and the resource tests; the 2*mu register widths/offsets are unchanged. Suite: 26 passed on 0.15.1/qpp-cpu. Signed-off-by: Scott Thornton <wsttiger@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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

kvmto reviewed Sep 28, 2026

View reviewed changes

kvmto approved these changes Oct 6, 2026

View reviewed changes

wsttiger mentioned this pull request Oct 7, 2026

Add CUDA-Q Algorithms agent skill #46

Open

wsttiger merged commit 1a8882f into NVIDIA:main Oct 7, 2026
22 checks passed

wsttiger deleted the features/primitives_arithmetic branch October 7, 2026 16:45

wsttiger mentioned this pull request Oct 7, 2026

Add alias sampling #48

Merged

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kvmto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

kvmto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wsttiger commented Oct 4, 2026

Uh oh!

wsttiger commented Oct 6, 2026

Uh oh!

kvmto left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wsttiger commented Sep 8, 2026 •

edited

Loading

`cudaq.control` (L46 / test L30)

kvmto left a comment •

edited

Loading