Add Quantum Counting algorithm implementation#176
Conversation
|
Hi @axif0, thanks for the contribution! Updating from main should fix the readthedocs issue, but you likely need to run |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #176 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 30 32 +2
Lines 1220 1358 +138
Branches 156 174 +18
==========================================
+ Hits 1220 1358 +138 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sesmart
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Overall the notebook looks very nice, good edge cases, left some concerns below.
| controlled_matrix = np.kron(p0, id_matrix) + np.kron(p1, grover_unitary) | ||
|
|
||
| targets = [control] + list(target_qubits) | ||
| circ.unitary(matrix=controlled_matrix, targets=targets) |
There was a problem hiding this comment.
While this does work, unfortunately, this is not quite what we are looking for 🥲 - it would probably be okay for a black box oracle, but here we can actually build the circuits, though this can certainly be valuable for building an intuition and confirming our logic.
Can you maybe grab some of the circuits from the Grover's file?
There was a problem hiding this comment.
The Grover operator is now built entirely from circuit primitives: build_oracle() for the phase oracle and amplify() for diffusion, both from grovers_search.py. The unitary is only extracted (to_unitary()) for the controlled-G operation in QPE, which is necessary since implementing arbitrary controlled multi-gate circuits natively is non-trivial.
There was a problem hiding this comment.
I made an error in suggesting to remove the run function -> the more common pattern here is to have a run_algorithm function, so if you could restore that, it would be great. My apologies.
I think there are maybe two options here, and it should close it out. We would like for there to be an actual circuit, and I agree that while the controlled unitary is non trivial, here there might be a reasonable way.
(1) For this particular U, I think you might be able to leverage the nature of the oracle a bit more. You could drop the amplify function, and then just call the oracle for "1" + the relevant states. I.e. in the diffussion operator "00" -> "100", and in the oracle, "xx" -> "1xx". Then you just need to implement the controlled H, with a Z on the ancilla to correct the phase. Does this make sense? This of course leads to an exponentially deep circuit, but that's a known issue with QPE.
(2) If this doesn't work, you can call qiskit-braket-provider from the run function before execution. If possible, we would like to limit this to 1- and 2q unitaries, which have standard and near-optimal solutions.
There was a problem hiding this comment.
I've updated the PR to implement Option 1 (building the controlled-Grover operator from gate-level primitives). Here’s a summary of the changes:
Gate-Level Controlled Grover: I replaced theto_unitary()matrix approach with a circuit-based implementation.
- Made the oracle controlled by prepending "1" to the marked states (adding the QPE qubit as an extra control on the MCZ).
- Made the diffusion operator controlled using controlled-Hadamard gates (decomposed via
S·H·T·CX·T†·H·S†) and a controlled zero-oracle.
-
Restored
run_quantum_counting: Added it back to match the library'srun_algorithmpattern and exported it ininit.py. -
Updated Tests: Updated to use the new gate-level circuits and added tests for
run_quantum_counting. All tests and linting pass. -
Updated Notebook: Replaced direct
device.run()calls withrun_quantum_counting, regenerated the circuit diagrams to show the native gates, and updated the note to mention that the circuit is now actually more compatible with QPU execution."
There was a problem hiding this comment.
Very nice! Looks good, am just running the tests.
| Returns: | ||
| Circuit: The complete quantum counting circuit with result types. | ||
| """ | ||
| counting_circ.add_circuit( |
There was a problem hiding this comment.
Avoid mutating circuits by pattern like
new_circuit = Circuit().add_circuit(counting_circ)
...
…ng a new circuit.
|
@sesmart, @yitchen-tim hello, Is there anything else for improvement? |
| controlled_matrix = np.kron(p0, id_matrix) + np.kron(p1, grover_unitary) | ||
|
|
||
| targets = [control] + list(target_qubits) | ||
| circ.unitary(matrix=controlled_matrix, targets=targets) |
There was a problem hiding this comment.
I made an error in suggesting to remove the run function -> the more common pattern here is to have a run_algorithm function, so if you could restore that, it would be great. My apologies.
I think there are maybe two options here, and it should close it out. We would like for there to be an actual circuit, and I agree that while the controlled unitary is non trivial, here there might be a reasonable way.
(1) For this particular U, I think you might be able to leverage the nature of the oracle a bit more. You could drop the amplify function, and then just call the oracle for "1" + the relevant states. I.e. in the diffussion operator "00" -> "100", and in the oracle, "xx" -> "1xx". Then you just need to implement the controlled H, with a Z on the ancilla to correct the phase. Does this make sense? This of course leads to an exponentially deep circuit, but that's a known issue with QPE.
(2) If this doesn't work, you can call qiskit-braket-provider from the run function before execution. If possible, we would like to limit this to 1- and 2q unitaries, which have standard and near-optimal solutions.
…nting and add `run_quantum_counting` function.
|
|
||
| # Second set of controlled-H gates on search qubits | ||
| for sq in search_qubits: | ||
| _add_controlled_h(circ, control, sq) |
There was a problem hiding this comment.
Lastly, I think you need a Z gate here - grover is 2 ss* - I, but I believe the oracle here is I - 2ss*. Might have just missed it.
… inherent -1 global phase, removing the need for post-processing phase adjustments.
|
@sesmart, could you please share any suggestions? I’ve pushed the latest updates. |
|
Looks good to go, thanks @axif0 ! |
|
@axif0, can you please comment on one of the algorithms issues in the BDK? It needs to be assigned but you have to comment first. |
Issue #, if available:
#1193
Description of changes:
Add the Quantum Counting algorithm (Brassard et al., 1998) — uses QPE on the Grover operator to estimate the number of marked items in an unstructured search space. Includes source module, unit tests, tutorial notebook, and README entry.
Testing done:
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.