Add IQPE benchmark#806
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughAdds a new deterministic iterative quantum phase estimation (IQPE) circuit generator function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/mqt/bench/benchmarks/iqpe_exact.py`:
- Around line 41-44: The code currently calls random.seed(10) which mutates the
global RNG; replace this with a local RNG instance to avoid side effects by
creating rng = random.Random(10) and then use rng.getrandbits(num_qubits) in the
theta generation loop (i.e., replace random.getrandbits(num_qubits) with
rng.getrandbits(num_qubits) and remove random.seed(10)), keeping the same loop
logic that ensures theta != 0.
- Around line 20-34: The function create_circuit should guard against too-small
inputs: after you subtract the ancilla (num_qubits = num_qubits - 1) ensure
there are at least 1 data qubit (original num_qubits >= 2) to avoid
num_qubits==0 which makes random.getrandbits(0) and the while theta==0 loop
non-terminating; add a benchmark-specific check at the start of create_circuit
(e.g., if original num_qubits < 2: raise ValueError with a clear message) so
callers get a deterministic error instead of hanging.
- Around line 56-81: The measurement feedback uses classical bits mismatched to
where results are stored: qc.measure(q[0], c[k]) stores into c[k] but the
correction loop reads c[m_index] where m_index = num_qubits-1-i, causing
corrections to read future/unset bits; fix by storing the measurement to the
same index the feedback expects (replace qc.measure(q[0], c[k]) with
qc.measure(q[0], c[index])) or alternatively change m_index computation in the
correction loop to match c[k] storage (e.g., set m_index = k - 1 - i); update
qc.measure and/or m_index logic so qc.measure, the correction loop (with
qc.if_test), and indexing all refer to the same classical bit indices.
- Around line 35-37: The circuit currently allocates a fixed 2-qubit register
(q) causing qc.num_qubits to not match the input num_qubits; change the register
allocation to mirror qpeexact.py by creating the main quantum register with
QuantumRegister(num_qubits - 1, "q"), a single-qubit ancilla register (e.g.,
QuantumRegister(1, "anc")), keep the classical register as
ClassicalRegister(num_qubits, "c"), and construct the circuit with qc =
QuantumCircuit(q, anc, c, name="iqpeexact") so qc.num_qubits == num_qubits.
| q = QuantumRegister(2, "q") | ||
| c = ClassicalRegister(num_qubits, "c") | ||
| qc = QuantumCircuit(q, c, name="iqpeexact") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the create_circuit function and its contract
cat -n src/mqt/bench/benchmarks/__init__.py | head -100Repository: munich-quantum-toolkit/bench
Length of output: 3894
🏁 Script executed:
# Examine the full iqpe_exact.py file to understand the algorithm
cat -n src/mqt/bench/benchmarks/iqpe_exact.pyRepository: munich-quantum-toolkit/bench
Length of output: 3413
🏁 Script executed:
# Search for how other benchmarks use QuantumRegister and num_qubits
rg -A5 'def create_circuit' src/mqt/bench/benchmarks/ -g'*.py' | head -150Repository: munich-quantum-toolkit/bench
Length of output: 10853
🏁 Script executed:
# Check if num_qubits is used elsewhere in iqpe_exact.py
rg -n 'num_qubits' src/mqt/bench/benchmarks/iqpe_exact.pyRepository: munich-quantum-toolkit/bench
Length of output: 538
🏁 Script executed:
# Get the full create_circuit function in __init__.py
sed -n '96,130p' src/mqt/bench/benchmarks/__init__.pyRepository: munich-quantum-toolkit/bench
Length of output: 1565
🏁 Script executed:
# Search for validation in create_circuit
rg -A15 'def create_circuit' src/mqt/bench/benchmarks/__init__.pyRepository: munich-quantum-toolkit/bench
Length of output: 972
🏁 Script executed:
# Check if iqpe_exact circuit.num_qubits equals the input parameter
rg -B2 -A10 'qc.num_qubits|circuit_size.*>.*0' src/mqt/bench/benchmarks/__init__.pyRepository: munich-quantum-toolkit/bench
Length of output: 54
🏁 Script executed:
# Compare with another algorithm like DJ to understand the pattern
cat -n src/mqt/bench/benchmarks/dj.py | sed -n '20,50p'Repository: munich-quantum-toolkit/bench
Length of output: 1146
🏁 Script executed:
# Search for usages of create_circuit to understand the contract
rg -n 'create_circuit\(' -B2 -A3 src/mqt/bench/ --glob='*.py' | head -100Repository: munich-quantum-toolkit/bench
Length of output: 7056
🏁 Script executed:
# Look for any assertions or validations on circuit.num_qubits after creation
rg -n '\.num_qubits\s*(==|!=|<|>|<=|>=)' src/mqt/bench/ --glob='*.py' | head -50Repository: munich-quantum-toolkit/bench
Length of output: 54
🏁 Script executed:
# Check the test file to understand expected behavior
fd -e test.py -o -e _test.py | head -20Repository: munich-quantum-toolkit/bench
Length of output: 188
🏁 Script executed:
# Look at benchmark_generation.py to see what's done with the returned circuit
cat -n src/mqt/bench/benchmark_generation.py | sed -n '70,100p'Repository: munich-quantum-toolkit/bench
Length of output: 1791
🏁 Script executed:
# Search for any assertions or validations after circuit creation
rg -A10 'qc = create_circuit' src/mqt/bench/benchmark_generation.pyRepository: munich-quantum-toolkit/bench
Length of output: 487
🏁 Script executed:
# Check if there's a test file that validates the contract
find . -name '*test*.py' -type f | xargs grep -l 'iqpe\|create_circuit' 2>/dev/null | head -5Repository: munich-quantum-toolkit/bench
Length of output: 93
🏁 Script executed:
# Check qpeexact to compare - it should have similar issues if this is an algorithm property
cat -n src/mqt/bench/benchmarks/qpeexact.py | head -50Repository: munich-quantum-toolkit/bench
Length of output: 2080
🏁 Script executed:
# Check the test file for iqpe_exact tests
grep -n 'iqpe' tests/test_bench.py -iRepository: munich-quantum-toolkit/bench
Length of output: 54
🏁 Script executed:
# Look for any size validation or assertions in tests
cat -n tests/test_bench.py | head -100Repository: munich-quantum-toolkit/bench
Length of output: 3481
🏁 Script executed:
# Check if there's documentation or comments about the expected circuit size
rg -B5 -A5 'iqpe_exact|iqpeinexact' docs/ README* 2>/dev/null || echo "No docs found"Repository: munich-quantum-toolkit/bench
Length of output: 85
🏁 Script executed:
# Verify by checking qpeexact structure more carefully - how many total qubits?
python3 << 'EOF'
# Parse both files to show the total qubit count formula
import re
files = {
"qpeexact": "src/mqt/bench/benchmarks/qpeexact.py",
"iqpe_exact": "src/mqt/bench/benchmarks/iqpe_exact.py"
}
for name, path in files.items():
with open(path) as f:
content = f.read()
# Find the qubit allocation section
match = re.search(r'num_qubits = num_qubits - 1.*?qc = QuantumCircuit\([^)]+\)', content, re.DOTALL)
if match:
print(f"\n{name}:")
print(match.group(0))
EOFRepository: munich-quantum-toolkit/bench
Length of output: 158
🏁 Script executed:
# Check if iqpe_exact or iqpeinexact is in the SPECIAL_QUBIT_COUNTS
grep -E 'iqpe|iqpe_exact' tests/test_bench.pyRepository: munich-quantum-toolkit/bench
Length of output: 54
🏁 Script executed:
# Let's verify the actual total qubit count for iqpe_exact
python3 << 'EOF'
# Simulate what iqpe_exact.py does
def simulate_iqpe_exact(num_qubits_input):
num_qubits = num_qubits_input - 1
# QuantumRegister(2, "q") creates 2 qubits
q_qubits = 2
# ClassicalRegister doesn't add qubits, only classical bits
c_bits = num_qubits
# Total qubits in circuit
total = q_qubits
return total
# Test with typical input
for input_val in [3, 5, 10]:
result = simulate_iqpe_exact(input_val)
print(f"Input: {input_val}, Circuit qubits: {result}, Contract expects: {input_val}, Match: {result == input_val}")
EOFRepository: munich-quantum-toolkit/bench
Length of output: 262
🏁 Script executed:
# Compare with qpeexact
python3 << 'EOF'
def simulate_qpeexact(num_qubits_input):
num_qubits = num_qubits_input - 1
# QuantumRegister(num_qubits, "q")
q_qubits = num_qubits
# QuantumRegister(1, "psi")
psi_qubits = 1
# ClassicalRegister doesn't add qubits
total = q_qubits + psi_qubits
return total
for input_val in [3, 5, 10]:
result = simulate_qpeexact(input_val)
print(f"Input: {input_val}, Circuit qubits: {result}, Contract expects: {input_val}, Match: {result == input_val}")
EOFRepository: munich-quantum-toolkit/bench
Length of output: 260
🏁 Script executed:
# Verify iqpe_exact is tested and will fail the contract
rg -n 'iqpe' tests/test_bench.py -i -A3 -B3Repository: munich-quantum-toolkit/bench
Length of output: 54
🏁 Script executed:
# Check if there's an iqpeinexact as well
fd -name '*iqpe*' src/mqt/bench/benchmarks/Repository: munich-quantum-toolkit/bench
Length of output: 302
🏁 Script executed:
# Let's verify by actually running the test assertion logic
python3 << 'EOF'
# Simulate the test that will run on iqpe_exact
benchmark_name = "iqpe_exact"
SPECIAL_QUBIT_COUNTS = {
"shor": 18,
"hrs_cumulative_multiplier": 5,
"bmw_quark_copula": 4,
"cdkm_ripple_carry_adder": 4,
"draper_qft_adder": 4,
"full_adder": 4,
"multiplier": 4,
"modular_adder": 4,
"rg_qft_multiplier": 4,
"vbe_ripple_carry_adder": 4,
}
# This is what test_quantumcircuit_levels does
input_value = SPECIAL_QUBIT_COUNTS.get(benchmark_name, 3)
print(f"Benchmark: {benchmark_name}")
print(f"Input value (circuit_size): {input_value}")
# What iqpe_exact creates
num_qubits = input_value - 1 # line 34
q_qubits = 2 # QuantumRegister(2, "q") line 35
# Final qc has only 2 qubits
qc_num_qubits = 2
print(f"Circuit qubits allocated: {qc_num_qubits}")
print(f"Test expects qc.num_qubits == input_value: {qc_num_qubits} == {input_value}")
print(f"Test will PASS: {qc_num_qubits == input_value}")
EOFRepository: munich-quantum-toolkit/bench
Length of output: 223
🏁 Script executed:
# Verify iqpe_exact would be tested with default input
python3 << 'EOF'
# Check what benchmarks are available and would be tested
import sys
sys.path.insert(0, 'src')
from mqt.bench.benchmarks import get_available_benchmark_names
names = get_available_benchmark_names()
print("iqpe_exact in available benchmarks:", "iqpe_exact" in names)
print("Total benchmarks:", len(names))
# Show that it would use default=3 since not in SPECIAL_QUBIT_COUNTS
SPECIAL_QUBIT_COUNTS = {
"shor": 18,
"hrs_cumulative_multiplier": 5,
"bmw_quark_copula": 4,
"cdkm_ripple_carry_adder": 4,
"draper_qft_adder": 4,
"full_adder": 4,
"multiplier": 4,
"modular_adder": 4,
"rg_qft_multiplier": 4,
"vbe_ripple_carry_adder": 4,
}
print("iqpe_exact in SPECIAL_QUBIT_COUNTS:", "iqpe_exact" in SPECIAL_QUBIT_COUNTS)
print("Would use input_value =", SPECIAL_QUBIT_COUNTS.get("iqpe_exact", 3))
EOFRepository: munich-quantum-toolkit/bench
Length of output: 425
Fix circuit qubit allocation to match the benchmark contract.
The circuit allocates exactly 2 qubits regardless of the num_qubits parameter, violating the contract enforced by the test suite (tests/test_bench.py:95): assert qc.num_qubits == input_value. With default test input of 3 qubits, the circuit will have 2 qubits, causing test failure. Allocate num_qubits - 1 qubits for the main register and 1 for the ancilla at line 35, matching the pattern in qpeexact.py.
🤖 Prompt for AI Agents
In `@src/mqt/bench/benchmarks/iqpe_exact.py` around lines 35 - 37, The circuit
currently allocates a fixed 2-qubit register (q) causing qc.num_qubits to not
match the input num_qubits; change the register allocation to mirror qpeexact.py
by creating the main quantum register with QuantumRegister(num_qubits - 1, "q"),
a single-qubit ancilla register (e.g., QuantumRegister(1, "anc")), keep the
classical register as ClassicalRegister(num_qubits, "c"), and construct the
circuit with qc = QuantumCircuit(q, anc, c, name="iqpeexact") so qc.num_qubits
== num_qubits.
There was a problem hiding this comment.
@carlareciofdz the suggestion by @coderabbitai here is actually wrong. The circuit should only ever have two qubits. The input parameter to the create_circuit function only really configures the precision to be used.
I see two ways forward here:
- either we relax the test suite slightly so that it does not assume that the final circuit has
nqubits. - or we fix the number of qubits in the
create_circuitfunction to2; error for everything else; and introduce an additional parameterprecisionto the function that regulates the number of iterations etc.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's examine the test file to understand SPECIAL_QUBIT_COUNTS and how tests work
echo "=== Examining test_bench.py for SPECIAL_QUBIT_COUNTS and test structure ==="
cat tests/test_bench.py | grep -A 20 "SPECIAL_QUBIT_COUNTS"
echo -e "\n=== Looking at test_quantumcircuit_levels function ==="
sed -n '/def test_quantumcircuit_levels/,/^def /p' tests/test_bench.py | head -30
echo -e "\n=== Checking if any benchmarks have additional parameters beyond num_qubits ==="
rg 'def create_circuit\(' src/mqt/bench/benchmarks/ -A 1 | head -80
Length of output: 8063
🏁 Script executed:
#!/bin/bash
# Check the create_circuit signature in __init__.py to understand the contract
echo "=== Checking create_circuit function signature in __init__.py ==="
sed -n '/^def create_circuit/,/^def /p' src/mqt/bench/benchmarks/__init__.py | head -40
echo -e "\n=== Looking for benchmarks with extra parameters ==="
rg 'def create_circuit\([^)]*,[^)]*\)' src/mqt/bench/benchmarks/ -g '*.py'
Length of output: 2911
🏁 Script executed:
#!/bin/bash
# Let's see the actual validation in test_bench.py
echo "=== Full context of test_quantumcircuit_levels ==="
sed -n '/def test_quantumcircuit_levels/,/^def [a-z]/p' tests/test_bench.py
echo -e "\n=== Looking for iqpe_exact tests ==="
grep -n "iqpe" tests/test_bench.py
Length of output: 2124
@burgholzer Thank you for the correction! You're absolutely right - I misunderstood the algorithm. IQPE is iterative and only needs 2 qubits regardless of precision.
Looking at the codebase architecture, I recommend Option 2 (fix to 2 qubits + add precision parameter) because:
-
Maintains test contract consistency: The existing test suite asserts
qc.num_qubits == input_valuefor all benchmarks, and Option 2 preserves this uniform contract. -
Has clear precedent: Several benchmarks already use additional parameters beyond
num_qubits:qaoa:repetitions,seedgraphstate:degree,seeddj:balancedvqe_real_amp:entanglement,reps
-
More explicit and maintainable: Separating qubit count from precision makes the API clearer -
create_circuit("iqpe_exact", 2, precision=5)is self-documenting.
Concrete steps:
- Add
"iqpe_exact": 2toSPECIAL_QUBIT_COUNTSintests/test_bench.py - Change signature to:
def create_circuit(num_qubits: int, precision: int = 3) -> QuantumCircuit: - Validate
num_qubits == 2(raise ValueError otherwise) - Use
precisioninstead ofnum_qubits - 1for iterations (line 34, line 36) - Update
test_iqpe_exact_structureto callcreate_circuit("iqpe_exact", 2, precision=4)
This approach keeps the architecture clean and consistent across all benchmarks.
🐰💭
🧠 Learnings used
Learnt from: adnathanail
Repo: munich-quantum-toolkit/bench PR: 803
File: src/mqt/bench/benchmarks/shors_nine_qubit_code.py:306-313
Timestamp: 2026-01-20T01:27:48.658Z
Learning: In the munich-quantum-toolkit/bench repository, the central `create_circuit` function in `src/mqt/bench/benchmarks/__init__.py` validates that `circuit_size > 0` before calling benchmark-specific factory functions. Therefore, individual benchmark implementations only need to validate benchmark-specific constraints (e.g., divisibility requirements) and don't need to re-check for positive values.
| for k in range(num_qubits): | ||
| index = num_qubits - 1 - k | ||
| # We reset the ancillary qubit in order to reuse in each iteration | ||
| qc.reset(q[0]) | ||
| qc.h(q[0]) | ||
|
|
||
| # Controlled unitary. The angle is normalized from | ||
| # [0, 2] to [-1, 1], which minimize the errors because uses shortest rotations | ||
| angle = float((lam * (1 << index)) % 2) | ||
| if angle > 1: | ||
| angle -= 2 | ||
|
|
||
| # We use pi convention for simplicity | ||
| qc.cp(angle * np.pi, q[0], q[1]) | ||
|
|
||
| # We apply phase corrections based on previous measurements. | ||
| for i in range(k): | ||
| m_index = num_qubits - 1 - i | ||
| true_angle = -1.0 / (1 << (k - i)) | ||
|
|
||
| with qc.if_test((c[m_index], 1)): | ||
| qc.p(true_angle * np.pi, q[0]) | ||
|
|
||
| qc.h(q[0]) | ||
| # We measure and store the result for future corrections | ||
| qc.measure(q[0], c[k]) |
There was a problem hiding this comment.
Feedback reads the wrong classical bit indices.
Measurements are stored in c[k], but corrections read c[num_qubits-1-i]. For k > 0, this targets bits that haven’t been measured yet, so feedback is incorrect. Store measurements at c[index] (or adjust m_index accordingly).
🐛 Proposed fix (store measurements at the same index used by feedback)
- for i in range(k):
- m_index = num_qubits - 1 - i
+ for i in range(k):
+ m_index = num_qubits - 1 - i
true_angle = -1.0 / (1 << (k - i))
with qc.if_test((c[m_index], 1)):
qc.p(true_angle * np.pi, q[0])
@@
- qc.measure(q[0], c[k])
+ qc.measure(q[0], c[index])🤖 Prompt for AI Agents
In `@src/mqt/bench/benchmarks/iqpe_exact.py` around lines 56 - 81, The measurement
feedback uses classical bits mismatched to where results are stored:
qc.measure(q[0], c[k]) stores into c[k] but the correction loop reads c[m_index]
where m_index = num_qubits-1-i, causing corrections to read future/unset bits;
fix by storing the measurement to the same index the feedback expects (replace
qc.measure(q[0], c[k]) with qc.measure(q[0], c[index])) or alternatively change
m_index computation in the correction loop to match c[k] storage (e.g., set
m_index = k - 1 - i); update qc.measure and/or m_index logic so qc.measure, the
correction loop (with qc.if_test), and indexing all refer to the same classical
bit indices.
There was a problem hiding this comment.
@burgholzer, let me trace through the indexing to verify:
For num_qubits = 4 (after the decrement, so 3 classical bits):
Iteration k=0:
index = 2- No feedback loop (range(0) is empty)
- Measure into c[0]
Iteration k=1:
index = 1- Feedback tries to read:
m_index = 3 - 0 = 3→ reads c[3] (not measured yet!) - Measure into c[1]
Iteration k=2:
index = 0- Feedback tries to read:
i=0:m_index = 3→ reads c[3] (not measured yet!)i=1:m_index = 2→ reads c[2] (not measured yet!)
- Measure into c[2]
The issue is still present. Measurements are stored at c[0], c[1], c[2], ... but the feedback loop tries to read from c[num_qubits-1], c[num_qubits-2], ... which haven't been measured yet.
The fix is to store measurements at c[index] instead of c[k], so the sequence becomes:
- k=0: measure into c[2] (index=2)
- k=1: measure into c[1] (index=1), read from c[2] ✓
- k=2: measure into c[0] (index=0), read from c[2], c[1] ✓
This aligns measurements with the feedback loop's expectations.
🧠 Learnings used
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:0-0
Timestamp: 2025-12-10T02:20:01.189Z
Learning: In QIRProgramBuilder::measure(Value, int64_t), the intentional design prevents duplicate measurements: the early return when `registerResultMap.find(key)` succeeds avoids generating multiple `mz` calls to the same classical bit index and ensures output recording contains only one entry per index. This implements an "override" semantic where repeated measurements to the same resultIndex reuse the cached pointer without additional side effects.
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1283
File: src/qir/runtime/QIR.cpp:196-201
Timestamp: 2025-11-01T15:57:31.153Z
Learning: In the QIR runtime (src/qir/runtime/QIR.cpp), the PRX gate (__quantum__qis__prx__body) is an alias for the R gate (Phased X-Rotation) and should call runtime.apply<qc::R>(theta, phi, qubit), not runtime.apply<qc::RX>() which is a single-parameter rotation gate.
Learnt from: adnathanail
Repo: munich-quantum-toolkit/bench PR: 803
File: src/mqt/bench/benchmarks/shors_nine_qubit_code.py:306-313
Timestamp: 2026-01-20T01:27:48.658Z
Learning: In the munich-quantum-toolkit/bench repository, the central `create_circuit` function in `src/mqt/bench/benchmarks/__init__.py` validates that `circuit_size > 0` before calling benchmark-specific factory functions. Therefore, individual benchmark implementations only need to validate benchmark-specific constraints (e.g., divisibility requirements) and don't need to re-check for positive values.
carlareciofdz
force-pushed
the
carla-IQPE
branch
from
0ccfd24 to
97e019e
Compare
…nto carla-IQPE
…nto carla-IQPE
…nto carla-IQPE
…nto carla-IQPE
…nto carla-IQPE
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@tests/test_bench.py`:
- Around line 148-167: The test_iqpe_exact_structure docstring and assertion
message are inconsistent with the actual check for 10 if_else operations; update
the docstring to state "4 reset conditionals + 6 correction conditionals = 10
total" (or equivalent) and change the assertion error string in the
if_else_count check to "Expected 10 conditional operations (4 reset + 6
correction), found {if_else_count}" so the message matches the assertion that
if_else_count == 10; locate these edits around the test_iqpe_exact_structure
function and the if_else_count assertion that references qc.data / if_else.
- Line 163: Remove the redundant inline type comment on the ops assignment:
replace the line "ops = dict(ops) # type: dict[str, int]" with just "ops =
dict(ops)"; the dict conversion already yields a plain dict from the return of
qc.count_ops(), so delete the "# type: dict[str, int]" annotation to clean up
the code (referencing the ops variable and qc.count_ops() return).
|
@burgholzer @flowerthrower Dear Lukas and Patrick the errors from the rabbit ai are solved. Whenever you have time, you can review it. |
burgholzer
added
python
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
Many thanks for the contribution here!
I just had a quick look through the implementation and notices some things that could potentially be improved for this benchmark.
You'll find these suggestions in the comments inline with the code.
Additionally, it would be great if you could also add a corresponding changelog entry to the respective file 🙏🏼
Don't hesitate to reach out in case of questions!
| qc.measure(q[0], c[0]) | ||
| with qc.if_test((c[0], 1)): | ||
| qc.x(q[0]) |
There was a problem hiding this comment.
Qiskit has a dedicated reset instruction that can be used here.
Furthermore, the first reset can be avoided as the qubit will be initialized in |0>.
| q = QuantumRegister(2, "q") | ||
| c = ClassicalRegister(num_qubits, "c") | ||
| qc = QuantumCircuit(q, c, name="iqpeexact") |
There was a problem hiding this comment.
@carlareciofdz the suggestion by @coderabbitai here is actually wrong. The circuit should only ever have two qubits. The input parameter to the create_circuit function only really configures the precision to be used.
I see two ways forward here:
- either we relax the test suite slightly so that it does not assume that the final circuit has
nqubits. - or we fix the number of qubits in the
create_circuitfunction to2; error for everything else; and introduce an additional parameterprecisionto the function that regulates the number of iterations etc.
| for k in range(num_qubits): | ||
| index = num_qubits - 1 - k | ||
| # We reset the ancillary qubit in order to reuse in each iteration | ||
| qc.reset(q[0]) | ||
| qc.h(q[0]) | ||
|
|
||
| # Controlled unitary. The angle is normalized from | ||
| # [0, 2] to [-1, 1], which minimize the errors because uses shortest rotations | ||
| angle = float((lam * (1 << index)) % 2) | ||
| if angle > 1: | ||
| angle -= 2 | ||
|
|
||
| # We use pi convention for simplicity | ||
| qc.cp(angle * np.pi, q[0], q[1]) | ||
|
|
||
| # We apply phase corrections based on previous measurements. | ||
| for i in range(k): | ||
| m_index = num_qubits - 1 - i | ||
| true_angle = -1.0 / (1 << (k - i)) | ||
|
|
||
| with qc.if_test((c[m_index], 1)): | ||
| qc.p(true_angle * np.pi, q[0]) | ||
|
|
||
| qc.h(q[0]) | ||
| # We measure and store the result for future corrections | ||
| qc.measure(q[0], c[k]) |
There was a problem hiding this comment.
This algorithm will probably need a safeguard to limit it to something like 60 qubits because otherwise the shift operations are no longer well defined and will cause issues.
A corresponding check should be added, the docstrings should mention this limitation, and a test is probably also in order.
There was a problem hiding this comment.
This is only covering the exact version of iQPE. Is there any particular reason for that?
It could make sense to add one more parameter to the create_circuit function that controls whether the phase is exactly representable or not.
Then, the naming and docstrings can also be adjusted accordingly to refer to this benchmark as IQPE (without the exact or inexact).
| def test_iqpe_exact_structure() -> None: | ||
| """Test that the Iterative Quantum Phase Estimation code circuit has the expected structure. | ||
|
|
||
| Verifies (for a 5-qubit input): | ||
| - Quantum registers: 4 target qubits and 1 ancillary qubit | ||
| - Classical register: num_qubits - 1 (4 bits) | ||
| - 8 measurements (4 resets + 4 final measurements) | ||
| - 10 conditional operations (4 reset conditionals + 6 correction conditionals) | ||
| """ | ||
| qc = create_circuit("iqpe_exact", 5) | ||
|
|
||
| assert qc.num_qubits == 5 | ||
| assert qc.num_clbits == 4 | ||
|
|
||
| ops = qc.count_ops() | ||
| ops = dict(ops) # type: dict[str, int] | ||
| assert ops.get("measure") == 8 | ||
|
|
||
| if_else_count = sum(1 for inst in qc.data if inst.operation.name == "if_else") | ||
| assert if_else_count == 10, f"Expected 10 conditional operations, found {if_else_count}" |
There was a problem hiding this comment.
This feels a bit hard-coded to be honest. Can we generalize this slightly to cover a few more cases?
|
Dear @carlareciofdz, may I ask for the current status of this PR? |
3 participants
Description
Please include a summary of the change and, if applicable, which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes #(issue)
Checklist: