-
-
Notifications
You must be signed in to change notification settings - Fork 30
Added Dynamical QFT + Measurement #807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Added Dynamical QFT + Measurement #807
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a new Dynamical Quantum Fourier Transform + Measurement benchmark circuit, a corresponding test, and updates project metadata and changelog. The benchmark builds a circuit with Hadamards, mid-circuit measurements, and measurement-conditioned phase rotations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/mqt/bench/benchmarks/dynamical_qft.py`:
- Line 21: The description string passed to the register_benchmark decorator for
"dynamical_qft" contains a leading space; update the decorator usage (the
`@register_benchmark` call for "dynamical_qft") to remove the leading space so the
description reads "Dynamic Quantum Fourier Transformation and Measurement (DQFT
+ M)" instead of " Dynamic...".
In `@tests/test_bench.py`:
- Around line 227-234: Add a circuit name assertion to test_dynamical_qft to
match other tests: after creating qc via create_circuit("dynamical_qft", 5)
assert that qc.name (or the circuit's name property used elsewhere) equals
"dynamical_qft" to ensure consistency with tests like test_bv and
test_graphstate_seed; update the test_dynamical_qft function to include this
assertion immediately after the circuit is created.
- Around line 236-245: The test_dynamical_qft_gates currently only checks
Hadamard and measurement counts; add an assertion to verify the conditional
phase rotations by counting entries in qc.data where inst.operation.name is "p"
or the phase gate identifier used by create_circuit for dynamical_qft, and
assert that for num_qubits=5 the phase/controlled-phase gate count equals 10
(4+3+2+1+0). Locate the test function test_dynamical_qft_gates and use qc.data
iteration (same pattern as hadamard_count) to compute phase_count and add the
assert phase_count == 10 so the dynamical QFT's conditional rotations are
covered.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pyproject.toml`:
- Around line 22-24: The pyproject currently pins "qiskit[qasm3-import]>=2.0.0"
while the comment references fixes present in Qiskit 1.3.2; either lower the
minimum to the appropriate older version (e.g., "qiskit[qasm3-import]>=1.3.2" or
the exact earliest release containing the OpenQASM3 export fixes) or add a clear
justification next to the dependency line explaining why 2.0.0 is required
(mentioning the specific features like BoxOp or control-flow changes if relied
upon); update the dependency string "qiskit[qasm3-import]>=2.0.0" or the
adjacent comment accordingly.
| dependencies = [ | ||
| # Qiskit 1.3.2 contains some fixes for exporting OpenQASM 3 files | ||
| "qiskit[qasm3-import]>=1.3.2", | ||
| "qiskit[qasm3-import]>=2.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find the benchmark file referenced in the PR context
find . -name "pyproject.toml" -type f | head -5Repository: munich-quantum-toolkit/bench
Length of output: 88
🏁 Script executed:
# Look for benchmark files that might be related to this dependency change
git ls-files | grep -E "(bench|benchmark)" | head -20Repository: munich-quantum-toolkit/bench
Length of output: 807
🏁 Script executed:
# Check if there's a git log or PR description that shows what changed
git log --oneline --all | grep -i "qiskit\|dependency" | head -10Repository: munich-quantum-toolkit/bench
Length of output: 682
🏁 Script executed:
# Search for if_test or control-flow usage in Python files to see if it's actually needed
rg "if_test|control_flow|ControlFlowOp" --type pyRepository: munich-quantum-toolkit/bench
Length of output: 1313
🏁 Script executed:
# Check the current dependency specification
cat pyproject.toml | head -40Repository: munich-quantum-toolkit/bench
Length of output: 1285
🌐 Web query:
Qiskit if_test QuantumCircuit control flow introduction version release
💡 Result:
QuantumCircuit.if_test() is Qiskit’s circuit-level API for building an if control-flow block (it creates an IfElseOp with only a “true” body, typically via the with qc.if_test(...): builder/context-manager form). [1][2]
Introduced: as part of the control-flow builder interface in Qiskit Terra 0.19.0 (published December 6, 2021). [3]
Packaged into the metapackage: Qiskit 0.34 notes that QASM 3 control-flow instructions (including IfElseOp, which if_test builds) were introduced in Terra 0.19.0. [4]
How it’s documented/used today: IBM’s “Classical feedforward and control flow” guide describes if_test as the Qiskit SDK equivalent of the conditional if statement used for dynamic circuits. [2]
Sources: [1] QuantumCircuit.if_test API docs, [2] IBM guide on control flow, [3] Terra 0.19.0 release record (Zenodo), [4] Qiskit 0.34 release notes.
🌐 Web query:
Qiskit qasm3-import extra when was it introduced release notes
💡 Result:
The qasm3-import installation extra was introduced alongside Qiskit’s initial OpenQASM 3 importer support in Qiskit 0.40.0 (Terra 0.23.0), where qiskit.qasm3.load() / qiskit.qasm3.loads() first appeared in the release notes [1]. The corresponding API docs for that series state that using these functions requires installing qiskit_qasm3_import, and recommend doing so via pip install qiskit-terra[qasm3-import] [2].
Sources: [1], [2]
🌐 Web query:
Qiskit 1.3.2 2.0.0 release history control flow operations
💡 Result:
Timeline (Qiskit SDK)
- v1.3.2 — published 2025-01-20: a bug-fix patch release; its published changelog lists fixes (for example in commutation checking and QASM3 export) and does not call out new/changed control-flow operations. [1]
- v2.0.0 — released 2025-04-03 (major release announcement): introduces major changes and new features in the v2.x line. [4]
Control-flow–related history across these versions
What was already happening in the v1.3 line (relevant to 1.3.2 users)
In the v1.3 release notes, Qiskit deprecated the legacy conditional mechanism (Instruction.condition and Instruction.c_if()), and directed users toward control-flow ops (notably IfElseOp via QuantumCircuit.if_test), with a transpiler pass ConvertConditionsToIfOps mentioned for migration. [2]
What changed/arrived in v2.0.0 for control flow
In v2.0.0 release notes, Qiskit added/updated several control-flow related pieces, including:
- New control-flow op
BoxOp(OpenQASM 3 “box” concept) plusQuantumCircuit.box()API. [3] get_control_flow_name_mapping()helper mapping control-flow operation names (like"if_else") to their classes. [3]ContractIdleWiresInControlFlowtranspiler pass (optimizes/removes idle qubits within control-flow blocks when semantics allow). [3]- Behavior change:
DAGCircuit.control_flow_op_nodes()now always returns a list (possibly empty) instead ofNone. [3]
Sources: [1] Qiskit 1.3.2 Zenodo changelog record; [2] Qiskit 1.3 release notes (IBM Quantum docs); [3] Qiskit 2.0 release notes (IBM Quantum docs); [4] IBM blog post announcing Qiskit SDK v2.0 release.
🏁 Script executed:
# Check git history for pyproject.toml changes related to Qiskit version
git log --oneline -20 -- pyproject.tomlRepository: munich-quantum-toolkit/bench
Length of output: 100
🏁 Script executed:
# Look for any recent commits that bumped Qiskit to 2.0.0
git log -p --all -S "qiskit\[qasm3-import\]>=2.0.0" -- pyproject.toml | head -100Repository: munich-quantum-toolkit/bench
Length of output: 54
🏁 Script executed:
# Check if there are any comments or issues referencing the 2.0.0 bump
git log --all --grep="2.0.0\|qiskit" --oneline | head -20Repository: munich-quantum-toolkit/bench
Length of output: 1521
Lower the Qiskit minimum version or justify the 2.0.0 requirement.
The comment states "Qiskit 1.3.2 contains some fixes for exporting OpenQASM 3 files," yet the requirement is >=2.0.0. The codebase uses only if_test (available since 0.19.0) and qasm3-import (0.40.0+), which are fully supported in 1.3.2. The 2.0.0 release adds features like BoxOp, get_control_flow_name_mapping, and control-flow behavior changes that are not used here. Either lower the minimum to 1.3.2 (or the earliest version that includes the specific QASM3 export fixes mentioned) or document why 2.0.0 is truly required.
🤖 Prompt for AI Agents
In `@pyproject.toml` around lines 22 - 24, The pyproject currently pins
"qiskit[qasm3-import]>=2.0.0" while the comment references fixes present in
Qiskit 1.3.2; either lower the minimum to the appropriate older version (e.g.,
"qiskit[qasm3-import]>=1.3.2" or the exact earliest release containing the
OpenQASM3 export fixes) or add a clear justification next to the dependency line
explaining why 2.0.0 is required (mentioning the specific features like BoxOp or
control-flow changes if relied upon); update the dependency string
"qiskit[qasm3-import]>=2.0.0" or the adjacent comment accordingly.
|
@flowerthrower @burgholzer Dear Lukas and Patrick, could you please review my code? Thanks in advance! |
Description
Adds the Dynamical QFT + Measurement circuit as a dynamical benchmarking circuit.
Fixes #798
Checklist: