Migrate synth_mcx_{1,2}_kg24 to Rust - #16783
Conversation
|
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the following people are relevant to this code:
|
ShellyGarion
left a comment
There was a problem hiding this comment.
Thanks @ellabarkan for the contribution! the PR looks good, I have some minor comments on the documentation.
| - | | ||
| Improved the runtime performance of :py:func:`~qiskit.synthesis.synth_mcx_1_kg24` and | ||
| :py:func:`~qiskit.synthesis.synth_mcx_2_kg24` (and their ``_clean``/``_dirty`` variants) | ||
| by migrating the implementation to Rust. These functions synthesize multi-controlled X |
There was a problem hiding this comment.
the functions ~qiskit.synthesis.synth_mcx_1_kg24 and ~qiskit.synthesis.synth_mcx_2_kg24 are only internal (and not part of the public API), so they should not be mentioned in the release notes.
There was a problem hiding this comment.
also you can write :func: and not :py:func:
| // Step 1: turn the ancilla into a "conditionally clean" qubit holding | ||
| // AND(control_0, control_1). RCCX is used (rather than CCX) because its stray | ||
| // relative phase is harmless: it will cancel against the same RCCX's inverse | ||
| // in step 5. |
There was a problem hiding this comment.
thanks for the explanation into steps. could you put the ref to the relevant part of the paper? or image?
| Ok((qc, leftover_ctrls)) | ||
| } | ||
|
|
||
| /// Flips `target` iff AND(ancilla0, leftover_ctrls...) holds (Step 3 of [1]). |
There was a problem hiding this comment.
where is Step 3 of [1]? Step 3 of which part of the paper?
| // Step 1: prime -- turn ancilla0 into a conditionally clean qubit holding | ||
| // AND(control_0, control_1). RCCX is used (rather than CCX) because its stray | ||
| // relative phase is harmless: it will cancel against the same RCCX's inverse | ||
| // in step 5. |
There was a problem hiding this comment.
again, could you refer to the specific part of the paper with the steps?
There was a problem hiding this comment.
perhaps it's worth to update this docstring (of synth_mcx_1_kg24) similarly to synth_mcx_2_kg24 ?
davidfcohen
left a comment
There was a problem hiding this comment.
Hello! I'd like to help make your code a little bit more Rusty.
NIT: I always lean toward intuitively named functions and variable names over many inline comments. Knowing nothing about the algorithm, this could be harder than I'm thinking.
…on compared to python
…CCX/RCCX from literal gates to decomposed operations
Coverage Report for CI Build 32118024013Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.004%) to 87.793%Details
Uncovered Changes
Coverage Regressions20 previously-covered lines in 5 files lost coverage.
Coverage Stats
💛 - Coveralls |
|
@ShellyGarion I have resolved the issue with the differences in circuit depth in log_depth_ladder implementation (2 ancilla quibits version) and updated the docstings in python API as we discussed |
Summary
Migrates
synth_mcx_1_kg24(1 ancilla, linear depth) andsynth_mcx_2_kg24(2 ancillae,logarithmic depth) from pure-Python
QuantumCircuitconstruction to Rust. Both functions implement theKhattar-Gidney 2024 conditionally-clean-ancilla constructions. No API changes were required.
Changes Made
Implementation:
synth_mcx_1_kg24andsynth_mcx_2_kg24(including helperslinear_depth_ladder_ops,log_depth_ladder_ops, andsynth_mcx_2_finish) incrates/synthesis/src/multi_controlled/mcx.rsqiskit._accelerate.synthesis.multi_controlled(py_synth_mcx_1_kg24,py_synth_mcx_2_kg24)synth_mcx_1_kg24,synth_mcx_1_clean_kg24,synth_mcx_1_dirty_kg24,synth_mcx_2_kg24,synth_mcx_2_clean_kg24,synth_mcx_2_dirty_kg24) now delegate to Rust viaQuantumCircuit._from_circuit_data(...)Algorithm Details:
clean with one RCCX on the first two controls, fold the remaining controls through a
linear-depth ladder of RCCX+X gates, apply one CCX to the target, then unfold/unprime. For
a dirty ancilla, the fold → CCX → unfold portion repeats a second time (toggle detection);
priming/unpriming happens once.
are folded through a log-depth binary AND-tree (successive rounds of pairwise RCCX gates
that double the pool of reusable conditionally-clean qubits), giving
O(log k)depth. Thefinish step is either a single CCX or a recursive call into the 1-ancilla construction over
the leftover controls plus a second ancilla.
Reference:
Khattar and Gidney, Rise of conditionally clean ancillae for optimizing quantum circuits,
arXiv:2407.17966
Performance
Benchmarking compares the Rust implementation (this branch) against the original Python
version (main branch), measured with
timeit.repeat(see methodology below).synth_mcx_1_kg24— clean ancilla:synth_mcx_1_kg24— dirty ancilla:synth_mcx_2_kg24— clean ancilla:synth_mcx_2_kg24— dirty ancilla:Result: The 1-ancilla construction delivers a consistent 2.2–3.0x speedup across all
tested control counts (k = 50–1000) in both clean and dirty modes. The 2-ancilla construction
shows a larger, scale-dependent gain: 2.5–4.2x for clean ancilla and 3.9–6.1x for dirty
ancilla, with the largest improvements at small k — consistent with the Python version's
relatively higher per-round
composeoverhead in the log-depth construction shrinking fasterthan the absolute operation count grows.
Benchmark Methodology
The code for performance estimation:
AI/LLM disclosure