✨ Add Structured Program Benchmarks #708
Conversation
📝 WalkthroughPre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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: 13
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
src/mqt/bench/.gitignore(1 hunks)src/mqt/bench/benchmarks/structured/ghz/ghz.qasm(1 hunks)src/mqt/bench/benchmarks/structured/grover/grover.qasm(1 hunks)src/mqt/bench/benchmarks/structured/iqft/iqft.qasm(1 hunks)src/mqt/bench/benchmarks/structured/iqpe/iqpe.qasm(1 hunks)src/mqt/bench/benchmarks/structured/multiplexer/multiplexer.qasm(1 hunks)src/mqt/bench/benchmarks/structured/qft/qft.qasm(1 hunks)src/mqt/bench/benchmarks/structured/qpe/qpe.qasm(1 hunks)src/mqt/bench/benchmarks/structured/teleportation/teleportation.qasm(1 hunks)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
🔇 Additional comments (4)
src/mqt/bench/benchmarks/structured/qpe/qpe.qasm (1)
1-26: Verify completeness of the QPE algorithm.The file appears to end after implementing only the reverse QFT portion of the QPE algorithm. Typical QPE implementations include:
- State preparation and controlled-U operations (present: lines 9-15)
- Inverse QFT (present: lines 18-26)
- Measurement and post-processing (missing)
While this may be intentionally abbreviated for benchmarking purposes, please confirm whether this represents the complete intended implementation or if additional steps should be added.
Additionally, verify the swap loop range on line 18:
for int i in [0:(n-2)/2]. If n=3, then (n-2)/2 = 0 (integer division), resulting in range [0:0] which produces no iterations. Confirm this is the intended behavior for small n values.src/mqt/bench/benchmarks/structured/multiplexer/multiplexer.qasm (2)
11-11: Verify runtime-sized qubit register declaration.Similar to the concern with line 8, this declares a qubit register with size
num_controls, which is computed at runtime. Verify that the target OpenQASM 3.0 parsers support this syntax.
7-7: Inconsistent AI summary.The AI summary states that
num_controlsis declared asconst int, but the code shows it asint(line 7). The actual code takes precedence over the summary.src/mqt/bench/benchmarks/structured/grover/grover.qasm (1)
21-21: Out-of-bounds array access on line 21.The register
qhas size n-1 (indices 0 through n-2), but this line attempts to accessq[n-1]as a target qubit, which is out of bounds. Additionally, the control registerq[0:n-2]would only have n-2 qubits, but the gate modifier specifiesctrl(n-2), requiring n-2 control qubits.Apply this diff to fix the indexing:
- ctrl(n-2) @ z q[0:n-2], q[n-1]; + ctrl(n-2) @ z q[0:n-3], q[n-2];This uses qubits 0 through n-3 as controls (n-2 qubits total) and qubit n-2 as the target.
Likely an incorrect or invalid review comment.
| @@ -0,0 +1 @@ | |||
| !*.qasm | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Negation won’t catch nested .qasm files
!*.qasm only re-includes files directly under this folder, not nested ones (e.g., benchmarks/structured/**). If the parent .gitignore ignores *.qasm, nested files remain ignored. Use a recursive pattern.
-!*.qasm
+!*.qasm
+!**/*.qasmIf the parent doesn’t ignore *.qasm, consider removing this file altogether to avoid confusion.
🏁 Script executed:
#!/usr/bin/env bash
# Search for qasm ignore patterns in all .gitignore files
rg -n '\*\.qasm' -g '*.gitignore'Length of output: 54
Include nested .qasm files
Root .gitignore ignores *.qasm, so !*.qasm here only re-includes files directly under this folder. Add a recursive pattern:
!*.qasm
+!**/*.qasm📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| !*.qasm | |
| !*.qasm | |
| !**/*.qasm |
🤖 Prompt for AI Agents
In src/mqt/bench/.gitignore around lines 1 to 1, the file currently re-includes
only top-level .qasm files with "!*.qasm" but the repo root ignores "*.qasm"
recursively; update this file to re-include nested .qasm files by using a
recursive negation pattern (e.g., "!**/*.qasm") so that .qasm files in
subdirectories beneath src/mqt/bench are not excluded.
| input int n; | ||
|
|
||
| qubit[n] q; | ||
|
|
||
| h q[0]; | ||
| for int i in [1:n-1] { | ||
| cx q[i - 1], q[i]; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Guard against n == 0 (q[0] access)
Program assumes n ≥ 1 due to h q[0]. Add a precondition note or a runtime guard to avoid invalid indexing for n == 0.
Example guard:
input int n;
qubit[n] q;
+// Require n >= 1
+if (n == 0) {
+ // no-op or raise, depending on your bench conventions
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/mqt/bench/benchmarks/structured/ghz/ghz.qasm around lines 4 to 11, the
code unconditionally accesses q[0] and will fail when n == 0; add a guard so the
circuit and operations are only emitted when n >= 1 (for example, conditionally
declare/operate on qubits or return early if n == 0), or document a precondition
explicitly that n must be >= 1; ensure both the h q[0] and the for-loop body are
skipped when n == 0 to avoid invalid indexing.
| h q; | ||
| x flag; | ||
|
|
||
| int num_iterations = int(pi / 4 * 2**(n-1)**0.5); |
There was a problem hiding this comment.
Fix the operator precedence in the Grover iteration formula.
The expression 2**(n-1)**0.5 evaluates as 2^(sqrt(n-1)) due to operator precedence (exponentiation is right-associative), but the correct Grover formula requires sqrt(2^(n-1)) or 2^((n-1)/2).
For example, with n=5 (4 search qubits):
- Current:
2^(sqrt(4))=2^2= 4 iterations - Correct:
sqrt(2^4)=sqrt(16)= 4 iterations
While these happen to match for n=5, they diverge for other values:
- n=9: Current gives
2^(sqrt(8))≈ 5.66, Correct givessqrt(2^8)= 16
Apply this diff to fix the formula:
-int num_iterations = int(pi / 4 * 2**(n-1)**0.5);
+int num_iterations = int(pi / 4 * sqrt(2**(n-1)));Or equivalently:
-int num_iterations = int(pi / 4 * 2**(n-1)**0.5);
+int num_iterations = int(pi / 4 * 2**((n-1)/2.0));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| int num_iterations = int(pi / 4 * 2**(n-1)**0.5); | |
| int num_iterations = int(pi / 4 * sqrt(2**(n-1))); |
🤖 Prompt for AI Agents
In src/mqt/bench/benchmarks/structured/grover/grover.qasm around line 12, the
Grover iteration formula uses wrong operator precedence: replace int
num_iterations = int(pi / 4 * 2**(n-1)**0.5); with an expression that computes
the square root of 2^(n-1) — e.g., use int num_iterations = int(pi / 4 *
(2**(n-1))**0.5); or int num_iterations = int(pi / 4 * 2**((n-1)/2)); so the
exponentiation groups correctly and yields sqrt(2^(n-1)).
|
|
||
| for int i in [1:num_iterations] { | ||
| // oracle | ||
| ctrl(n-1) @ z q[0:n-1], flag; |
There was a problem hiding this comment.
Out-of-bounds array access on line 16.
The register q is declared as qubit[n-1] on line 6, which creates a register with indices 0 through n-2. Accessing q[0:n-1] attempts to access index n-1, which is out of bounds.
Apply this diff to fix the indexing:
- ctrl(n-1) @ z q[0:n-1], flag;
+ ctrl(n-1) @ z q, flag;Note: In OpenQASM 3.0, using the register name q without explicit indices applies the gate to all qubits in the register.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ctrl(n-1) @ z q[0:n-1], flag; | |
| ctrl(n-1) @ z q, flag; |
🤖 Prompt for AI Agents
In src/mqt/bench/benchmarks/structured/grover/grover.qasm around line 16, the
slice q[0:n-1] is out-of-bounds because q is declared qubit[n-1] (indices
0..n-2); replace the indexed slice with the register name q (or use q[0:n-2] if
an explicit range is desired) so the controlled Z gate applies to the valid
qubits of the register.
|
|
||
| for int i in [0:n-1] { | ||
| for int j in [0:i-1] { | ||
| if (res[n - 1 - j]) { |
There was a problem hiding this comment.
Use explicit comparison for bit conditions
Make bit checks explicit for broader parser compatibility.
- if (res[n - 1 - j]) {
+ if (res[n - 1 - j] == 1) {🤖 Prompt for AI Agents
In src/mqt/bench/benchmarks/structured/iqft/iqft.qasm around line 11, the
conditional if (res[n - 1 - j]) relies on truthiness; change it to an explicit
comparison (e.g., compare to 1) so the bit check is unambiguous and compatible
with stricter parsers — replace the truthy check with an explicit equality check
against the bit value.
| int num_controls = n - 1; | ||
| input angle[2**num_controls] angles; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify OpenQASM 3.0 support for runtime-sized array declarations.
Line 8 declares an input array with size 2**num_controls, where num_controls is computed at runtime from the input parameter n. OpenQASM 3.0 typically requires array dimensions to be compile-time constants. This may cause parsing errors when the benchmark is compiled.
Run the following verification to check OpenQASM 3.0 specification and test with available parsers:
🌐 Web query:
Does OpenQASM 3.0 support array declarations with runtime-computed sizes like "input angle[2**num_controls] angles" where num_controls is derived from an input parameter?
💡 Result:
Short answer: No — OpenQASM 3.0 requires array/register sizes to be compile‑time constants. You cannot declare a global array with a size computed only at runtime (e.g. input angle[2**num_controls] where num_controls is a non-const input). See the specification passages that sizes must be compile‑time const and examples showing invalid runtime-sized declarations. [1][2]
References
- Types & compile‑time constants (array / qubit sizes must be const). [1]
- Subroutines: array parameters can have unspecified runtime lengths for passing slices/references, but that does not allow runtime-sized declarations at global scope. [2]
Require compile-time constant for array dimensions
Lines 7–8: num_controls = n - 1 isn’t constant, so input angle[2**num_controls] angles; violates OpenQASM 3.0. Either make n a const parameter or refactor to use an unsized array (input angle[] angles;) inside a subroutine.
🤖 Prompt for AI Agents
In src/mqt/bench/benchmarks/structured/multiplexer/multiplexer.qasm around lines
7–8, the dimension expression `2**num_controls` uses `num_controls = n - 1`
which is not a compile-time constant and violates OpenQASM 3.0; either change
`n` to a const parameter so `num_controls` becomes a compile-time constant
(e.g., declare `const int n = ...` or `const int num_controls = ...`) and keep
the sized input, or refactor to accept an unsized input array and use a
subroutine signature like `input angle[] angles;` and pass a properly sized
array at call site so the file no longer requires non-constant array dimensions.
|
|
||
| qubit[num_controls] controls; | ||
| qubit target; | ||
| bit[num_controls + 1] c; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused classical bit register.
The classical bit register c is declared but never used in the program. Consider removing it to improve code clarity.
Apply this diff:
qubit[num_controls] controls;
qubit target;
-bit[num_controls + 1] c;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bit[num_controls + 1] c; | |
| qubit[num_controls] controls; | |
| qubit target; |
🤖 Prompt for AI Agents
In src/mqt/bench/benchmarks/structured/multiplexer/multiplexer.qasm around line
13, the classical bit register `c` (bit[num_controls + 1] c;) is declared but
never referenced; remove this unused declaration from the QASM file to eliminate
the unused register and improve clarity, ensuring no other code depends on `c`
before committing the change.
| for int i in [0:(n-1)/2] { | ||
| swap q[i], q[n - 1 - i]; | ||
| } |
There was a problem hiding this comment.
Fix swap loop bound to avoid non-integer/end-inclusive pitfalls
(n-1)/2 can be non-integer; inclusivity may overrun. Prefer a bound that clearly iterates floor(n/2) swaps.
-for int i in [0:(n-1)/2] {
+for int i in [0:n/2 - 1] {
swap q[i], q[n - 1 - i];
}Alternatively (more robust to integer semantics):
-for int i in [0:(n-1)/2] {
- swap q[i], q[n - 1 - i];
-}
+for int i in [0:n-1] {
+ if (i < n - 1 - i) {
+ swap q[i], q[n - 1 - i];
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for int i in [0:(n-1)/2] { | |
| swap q[i], q[n - 1 - i]; | |
| } | |
| for int i in [0:n/2 - 1] { | |
| swap q[i], q[n - 1 - i]; | |
| } |
| for int i in [0:(n-1)/2] { | |
| swap q[i], q[n - 1 - i]; | |
| } | |
| for int i in [0:n-1] { | |
| if (i < n - 1 - i) { | |
| swap q[i], q[n - 1 - i]; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/mqt/bench/benchmarks/structured/qft/qft.qasm around lines 14 to 16, the
swap loop uses the bound (n-1)/2 which can be non-integer and may be interpreted
inclusively, risking wrong swap count or out-of-range indices; change the loop
to iterate exactly floor(n/2) times — e.g. use an explicit integer upper bound
like [0:floor(n/2)-1] or rewrite as a half-open form using i < n/2 so the loop
runs for i = 0 .. floor(n/2)-1 and performs exactly floor(n/2) swaps without
overrun.
| if (b1) { | ||
| x q2; | ||
| } | ||
| if (a) { | ||
| z q2; | ||
| } |
There was a problem hiding this comment.
Use explicit comparisons for bit-controlled corrections
Ensure conditionals on measured bits are explicit.
-if (b1) {
+if (b1 == 1) {
x q2;
}
-if (a) {
+if (a == 1) {
z q2;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (b1) { | |
| x q2; | |
| } | |
| if (a) { | |
| z q2; | |
| } | |
| if (b1 == 1) { | |
| x q2; | |
| } | |
| if (a == 1) { | |
| z q2; | |
| } |
🤖 Prompt for AI Agents
In src/mqt/bench/benchmarks/structured/teleportation/teleportation.qasm around
lines 24 to 29, the conditionals use implicit truthiness (if (b1) and if (a));
change them to explicit comparisons against the measured-bit value (for example
if (b1 == 1) { x q2; } and if (a == 1) { z q2; }) so the bit-controlled
corrections are unambiguous and robust.
| z q2; | ||
| } | ||
|
|
||
| // Now q2 is in the previous state of a. |
There was a problem hiding this comment.
Comment nit: variable name
“previous state of a” should be “previous state of psi”.
-// Now q2 is in the previous state of a.
+// Now q2 is in the previous state of psi.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Now q2 is in the previous state of a. | |
| // Now q2 is in the previous state of psi. |
🤖 Prompt for AI Agents
In src/mqt/bench/benchmarks/structured/teleportation/teleportation.qasm around
line 31, the inline comment uses “previous state of a” which is incorrect;
update the comment to read “previous state of psi” so it accurately refers to
the state variable psi.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Description
Currently, MQT Bench only supports simple quantum circuits. However, recently, the topic of structured quantum programs, as opposed to straight-line circuits is gaining popularity.
Compared to normal circuits, these structured programs also support structured control flow such as
forloops,ifdirectives and more.However, a lot of compilers are still completely incompatible with this type of program.
For instance, while Qiskit generally allows parsing OpenQASM3 inputs (which do support structured control flow), Qiskit's OpenQASM parser will fail if the structured control flow is no longer trivial (i.e. the loop variable may be used as an index for gates).
Therefore, we want to collect a set of programs that are based on quantum programs that actually appear in reality, and provide them as a challenge for future compiler developers. The employed programs are taken from the discussion in the corresponding tracking issue #691.
Checklist:
Summary by CodeRabbit
New Features
Chores