Fix classically controlled multiline QASM output - #8376
akshaysoftware wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
arettig
left a comment
arettig
left a comment
There was a problem hiding this comment.
Thanks for doing this! Just a couple minor comments below.
|
|
||
|
|
||
| def test_qasm_multiline_classically_controlled_operation_qasm2() -> None: | ||
| q0, q1, q2, q3 = cirq.LineQubit.range(4) |
There was a problem hiding this comment.
nit: This test could be done with 3 qubits if you move the measurement gate.
|
|
||
|
|
||
| def test_qasm_multiline_classically_controlled_operation_qasm3() -> None: | ||
| q0, q1, q2, q3 = cirq.LineQubit.range(4) |
There was a problem hiding this comment.
nit: same as above, could be done in 3 qubits.
| condition_qasm = " && ".join(protocols.qasm(c, args=args) for c in self._conditions) | ||
| return f'if ({condition_qasm}) {subop_qasm}' | ||
|
|
||
| if subop_qasm.count('\n') <= 1: |
There was a problem hiding this comment.
Technically you could have multiple gates on a single line which would break this. It looks like no built-in gates do this, so it's only a problem for user-defined gates.
Relatedly, if subop_qasm is only whitespace, this will output a hanging if statement that applies to whatever comes next.
Maybe count the number of semicolons instead?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8376 +/- ##
==========================================
- Coverage 99.59% 99.59% -0.01%
==========================================
Files 1131 1131
Lines 103591 103607 +16
==========================================
+ Hits 103170 103185 +15
- Misses 421 422 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Fixes #8329.
Updates
ClassicallyControlledOperation._qasm_so multi-statement QASM output is handled correctly:Adds regression tests for both QASM 2.0 and 3.0 using classically controlled
CCZ.