Correctly handle non-UnitaryGate gates named "unitary" (#14109)

Normally creating a custom gate class that overloads the name of a
Qiskit defined operation is not valid and not allowed. The names have
meaning and are often used as identifiers and this overloading the name
will prevent Qiskit from correctly identifying an operation. However as
was discovered in #14103 there are some paths available around
serialization and I/O where Qiskit does this itself. For example, qasm
(both 2 and 3) is a lossy serialization format and qasm2 doesn't have a
representation of a UnitaryGate. So when the qasm2 exporter encounteres
a `UnitaryGate` it is serialized as a custom gate definition with the
name "unitary" in the output qasm2 and the definition is a decomposition
of the unitary from the `UnitaryGate`. When that qasm2 program is
subsequently deserialized by qiskit parser the custom gate named "unitary"
is added as a `_DefinedGate` subclass which includes an `__array__`
implementation which computes the unitary from the definition using
the quantum info Operator class. This makes the custom gate parsed from
qasm2 look like a `UnitaryGate` despite not actually one so this is
typically fine for most use cases. However, since #13759 trying to add
that not `UnitaryGate` object named "unitary" would cause the Python ->
Rust translation to panic (which happens as part of qasm2
desierailzation). because the conversion was expecting a gate named
`unitary` to be a `UnitaryGate` as is prescribed by the data model.

This commit fixes this by gracefully handling the lack of a matrix
parameter as it not actually being a `UnitaryGate` and instead the
object gets treated as a `PyGate` in rust which is the expected
behavior.

Related to #14103

Author: mtreinish

crates/circuit/src/circuit_instruction.rs

@@ -578,7 +578,7 @@ impl<'py> FromPyObject<'py> for OperationFromPython {
         // We need to check by name here to avoid a circular import during initial loading
         if ob.getattr(intern!(py, "name"))?.extract::<String>()? == "unitary" {
             let params = extract_params()?;
-            if let Param::Obj(data) = &params[0] {
+            if let Some(Param::Obj(data)) = params.first() {
                 let py_matrix: PyReadonlyArray2<Complex64> = data.extract(py)?;
                 let matrix: Option<MatrixView2<Complex64>> = py_matrix.try_as_matrix();
                 if let Some(x) = matrix {

test/python/qasm2/test_structure.py

@@ -1836,3 +1836,19 @@ def test_single_quoted_path(self):
         qc = QuantumCircuit(QuantumRegister(1, "q"))
         qc.h(0)
         self.assertEqual(parsed, qc)
+
+    def test_unitary_qasm(self):
+        """Test that UnitaryGate can be loaded by OQ2 correctly."""
+        qc = QuantumCircuit(1)
+        qc.unitary([[1, 0], [0, 1]], 0)
+        qasm = """
+            OPENQASM 2.0;
+            include "qelib1.inc";
+            gate unitary q0 { U(0,0,0) q0; }
+            qreg q[1];
+            unitary q[0];
+        """
+        parsed = qiskit.qasm2.loads(qasm)
+        self.assertIsInstance(parsed, QuantumCircuit)
+        self.assertIsInstance(parsed.data[0].operation, qiskit.qasm2.parse._DefinedGate)
+        self.assertEqual(Operator.from_circuit(parsed), Operator.from_circuit(qc))

test/python/transpiler/test_split_2q_unitaries.py

@@ -13,6 +13,9 @@
 """
 Tests for the Split2QUnitaries transpiler pass.
 """
+
+import io
+
 from math import pi
 from test import QiskitTestCase
 import numpy as np
@@ -26,6 +29,7 @@
 from qiskit.quantum_info.operators.predicates import matrix_equal
 from qiskit.transpiler.passes import Collect2qBlocks, ConsolidateBlocks
 from qiskit.transpiler.passes.optimization.split_2q_unitaries import Split2QUnitaries
+from qiskit import qpy
 
 
 class TestSplit2QUnitaries(QiskitTestCase):
@@ -377,3 +381,41 @@ def test_2q_swap_with_large_circuit(self):
         )  # the original 2-qubit unitary should be split into 2 1-qubit unitaries.
         self.assertTrue(expected_op.equiv(res_op))
         self.assertTrue(matrix_equal(expected_op.data, res_op.data, ignore_phase=False))
+
+    def test_overloaded_unitary_name_from_qasm(self):
+        """Test that an otherwise invalid custom gate named unitary created via valid Qiskit
+        API calls doesn't crash the pass
+
+        See: https://github.com/Qiskit/qiskit/issues/14103
+        """
+
+        qasm_str = """OPENQASM 2.0;
+        include "qelib1.inc";
+        gate cx_o0 q0,q1 { x q0; cx q0,q1; x q0; }
+        gate unitary q0,q1 { u(pi/2,0.6763483147328913,0) q0; u(1.6719020266110614,-pi/2,0) q1; cx q0,q1; u(pi,-0.9111063207475532,3.1249343449042435) q0; u(1.6719020266110616,0,-pi/2) q1; }
+        qreg v__0__0_[0];
+        qreg l___0__0___1_[2];
+        qreg l___0__0___2_[2];
+        qreg v__0__1_[0];
+        qreg l___0__1___1_[2];
+        qreg v__1__0_[0];
+        qreg l___1__0___1_[2];
+        qreg l___1__0___2_[2];
+        qreg v__1__1_[0];
+        qreg l___1__1___1_[2];
+        creg meas[12];
+        unitary l___0__0___2_[0],l___0__1___1_[0];
+        """
+        # Parse qasm string to get custom unitary gate that's not a UnitaryGate (but has matrix defined)
+        qc = QuantumCircuit.from_qasm_str(qasm_str)
+        # Roundtrip QPY to lose the custom matrix definition from qasm2
+        # unitary
+        with io.BytesIO() as buf:
+            qpy.dump(qc, buf)
+            # Rewind back to the beginning of the "file".
+            buf.seek(0)
+            qc = qpy.load(buf)[0]
+        # Run split unitaries pass with custom gate named unitary that has
+        # no matrix.
+        res = Split2QUnitaries()(qc)
+        self.assertEqual(res, qc)