From 31f13c6bee1de07040b0315bfae200ef0ee52ed7 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Mon, 24 Jul 2023 21:04:05 +0100 Subject: [PATCH] Fix empty-barrier handling in OpenQASM 2 (#10469) The new parser would allow a `barrier;` statement, implicitly broadcasting it across all qubits in scope. This is technically not supported by the OpenQASM 2 specification, but is a useful quality-of-life extension to the specification (in the same way that Qiskit interprets barriers, and the OpenQASM 3 specification defines the `barrier;` statement). The precise rule is added to the new parser's `strict` mode. The OpenQASM 2 _exporter_ similarly should not have been putting out `barrier;` statements. These could only occur in Qiskit when a barrier was explicitly constructed with zero elements (as opposed to the call `QuantumCircuit.barrier()`, which has the all-in-scope behaviour), and consequently have no actual meaning or effect. The exporter is modified to simply skip such instructions, for as long as Qiskit permits the qubitless barrier statement. (cherry picked from commit e75893d455577b3d211f85b84bb93f69e54b435f) --- crates/qasm2/src/parse.rs | 9 +++++++++ qiskit/circuit/quantumcircuit.py | 4 ++++ ...m2-fix-zero-op-barrier-4af211b119d5b24d.yaml | 13 +++++++++++++ test/python/circuit/test_circuit_qasm.py | 17 +++++++++++++++++ test/python/qasm2/test_parse_errors.py | 7 +++++++ 5 files changed, 50 insertions(+) create mode 100644 releasenotes/notes/qasm2-fix-zero-op-barrier-4af211b119d5b24d.yaml diff --git a/crates/qasm2/src/parse.rs b/crates/qasm2/src/parse.rs index 9960258074a0..e4c749841124 100644 --- a/crates/qasm2/src/parse.rs +++ b/crates/qasm2/src/parse.rs @@ -1351,6 +1351,15 @@ impl State { } self.check_trailing_comma(comma.as_ref())?; qubits + } else if self.strict { + return Err(QASM2ParseError::new_err(message_generic( + Some(&Position::new( + self.current_filename(), + barrier_token.line, + barrier_token.col, + )), + "[strict] barrier statements must have at least one argument", + ))); } else if let Some(num_gate_qubits) = num_gate_qubits { (0..num_gate_qubits).map(QubitId::new).collect::>() } else { diff --git a/qiskit/circuit/quantumcircuit.py b/qiskit/circuit/quantumcircuit.py index 8727d78b810e..1d99ab34e679 100644 --- a/qiskit/circuit/quantumcircuit.py +++ b/qiskit/circuit/quantumcircuit.py @@ -1716,6 +1716,10 @@ def qasm( elif operation.name == "reset": instruction_qasm = f"reset {bit_labels[instruction.qubits[0]]};" elif operation.name == "barrier": + if not instruction.qubits: + # Barriers with no operands are invalid in (strict) OQ2, and the statement + # would have no meaning anyway. + continue qargs = ",".join(bit_labels[q] for q in instruction.qubits) instruction_qasm = "barrier;" if not qargs else f"barrier {qargs};" else: diff --git a/releasenotes/notes/qasm2-fix-zero-op-barrier-4af211b119d5b24d.yaml b/releasenotes/notes/qasm2-fix-zero-op-barrier-4af211b119d5b24d.yaml new file mode 100644 index 000000000000..9b86441fe814 --- /dev/null +++ b/releasenotes/notes/qasm2-fix-zero-op-barrier-4af211b119d5b24d.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + The OpenQASM 2 parser (:func:`.qasm2.load` and :func:`~.qasm2.loads`) running in ``strict`` mode + will now correctly emit an error if a ``barrier`` statement has no arguments. When running in + the (default) more permissive mode, an argument-less ``barrier`` statement will continue to + cause a barrier on all qubits currently in scope (the qubits a gate definition affects, or all + the qubits defined by a program, if the statement is in a gate body or in the global scope, + respectively). + - | + The OpenQASM 2 exporter (:meth:`.QuantumCircuit.qasm`) will now no longer attempt + to output ``barrier`` statements that act on no qubits. Such a barrier statement has no effect + in Qiskit either, but is invalid OpenQASM 2. diff --git a/test/python/circuit/test_circuit_qasm.py b/test/python/circuit/test_circuit_qasm.py index 83e1d7999a47..0aa6ac6ececc 100644 --- a/test/python/circuit/test_circuit_qasm.py +++ b/test/python/circuit/test_circuit_qasm.py @@ -829,6 +829,23 @@ def test_sequencial_inner_gates_with_same_name(self): self.assertEqual(qc.qasm(), expected_output) + def test_empty_barrier(self): + """Test that a blank barrier statement in _Qiskit_ acts over all qubits, while an explicitly + no-op barrier (assuming Qiskit continues to allow this) is not output to OQ2 at all, since + the statement requires an argument in the spec.""" + qc = QuantumCircuit(QuantumRegister(2, "qr1"), QuantumRegister(3, "qr2")) + qc.barrier() # In Qiskit land, this affects _all_ qubits. + qc.barrier([]) # This explicitly affects _no_ qubits (so is totally meaningless). + + expected = """\ +OPENQASM 2.0; +include "qelib1.inc"; +qreg qr1[2]; +qreg qr2[3]; +barrier qr1[0],qr1[1],qr2[0],qr2[1],qr2[2]; +""" + self.assertEqual(qc.qasm(), expected) + if __name__ == "__main__": unittest.main() diff --git a/test/python/qasm2/test_parse_errors.py b/test/python/qasm2/test_parse_errors.py index 1cc90cdaef3c..1963fc64b3b6 100644 --- a/test/python/qasm2/test_parse_errors.py +++ b/test/python/qasm2/test_parse_errors.py @@ -865,3 +865,10 @@ def test_required_version_empty(self): qiskit.qasm2.QASM2ParseError, r"\[strict\] .*needed a version statement" ): qiskit.qasm2.loads("", strict=True) + + def test_barrier_requires_args(self): + program = "OPENQASM 2.0; qreg q[2]; barrier;" + with self.assertRaisesRegex( + qiskit.qasm2.QASM2ParseError, r"\[strict\] barrier statements must have at least one" + ): + qiskit.qasm2.loads(program, strict=True)