Fix qpy custom ControlledGate with overloaded _define() (Qiskit#8927)

This commit fixes a bug in the QPY serialization of ControlledGate
subclasses that defined custom _define() methods. The _define() method
is the typical way to provide a custom definition in Gate classes. While
ControlledGate class provides an alternative interface that handles
custom control states at initialization, but the _define() interface is
still valid even if it doesn't account for this. In Qiskit#8571 we updated the
QPY serialization code to use the _definition() method directly to
correctly handle the open control case. But this fix neglected the case
where people were providing definitions via the mechanism from Gate.
This commit fixes this assumption by ensuring we load the definition
that comes from _define() which will fix the serialization of these
custom subclasses.

Fixes Qiskit#8794

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

qiskit/qpy/binary_io/circuits.py

@@ -632,6 +632,9 @@ def _write_custom_operation(file_obj, name, operation, custom_operations):
         # state is open, and the definition setter (during a subsequent read) uses the "fully
         # excited" control definition only.
         has_definition = True
+        # Build internal definition to support overloaded subclasses by
+        # calling definition getter on object
+        operation.definition  # pylint: disable=pointless-statement
         data = common.data_to_binary(operation._definition, write_circuit)
         size = len(data)
         num_ctrl_qubits = operation.num_ctrl_qubits

releasenotes/notes/fix-qpy-custom-controlled-gate-a9355df1a88a83a5.yaml

@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    Fixed an issue in QPY serialization (:func:`~.qpy.dump`) when a custom
+    :class:`~.ControlledGate` subclass that overloaded the ``_define()``
+    method to provide a custom definition for the operation. Previously,
+    this case of operation was not serialized correctly because it wasn't
+    accounting for using the potentially ``_define()`` method to provide
+    a definition.
+    Fixes `#8794 <https://github.com/Qiskit/qiskit-terra/issues/8794>`__

test/python/circuit/test_circuit_load_from_qpy.py

@@ -34,6 +34,7 @@
 from qiskit.test import QiskitTestCase
 from qiskit.circuit.qpy_serialization import dump, load
 from qiskit.quantum_info.random import random_unitary
+from qiskit.circuit.controlledgate import ControlledGate
 
 
 class TestLoadFromQPY(QiskitTestCase):
@@ -1055,6 +1056,34 @@ def test_standard_control_gates(self):
         new_circuit = load(qpy_file)[0]
         self.assertEqual(qc, new_circuit)
 
+    def test_controlled_gate_subclass_custom_definition(self):
+        """Test controlled gate with overloaded definition.
+
+        Reproduce from: https://github.com/Qiskit/qiskit-terra/issues/8794
+        """
+
+        class CustomCXGate(ControlledGate):
+            """Custom CX with overloaded _define."""
+
+            def __init__(self, label=None, ctrl_state=None):
+                super().__init__(
+                    "cx", 2, [], label, num_ctrl_qubits=1, ctrl_state=ctrl_state, base_gate=XGate()
+                )
+
+            def _define(self) -> None:
+                qc = QuantumCircuit(2, name=self.name)
+                qc.cx(0, 1)
+                self.definition = qc
+
+        qc = QuantumCircuit(2)
+        qc.append(CustomCXGate(), [0, 1])
+        qpy_file = io.BytesIO()
+        dump(qc, qpy_file)
+        qpy_file.seek(0)
+        new_circ = load(qpy_file)[0]
+        self.assertEqual(qc, new_circ)
+        self.assertEqual(qc.decompose(), new_circ.decompose())
+
     def test_load_with_loose_bits(self):
         """Test that loading from a circuit with loose bits works."""
         qc = QuantumCircuit([Qubit(), Qubit(), Clbit()])