Serializing/Deserializing a circuit with QPD gates causes a ValueError

[caleb-johnson]

ValueError: Missing \'basis_id\': unable to realize SingleQubitQPDGate is raised when serializing/deserializing a circuit with QPDGates.

This should not raise an error. The gate should be able to be serialized and deserialized without being decomposed into single-qubit Qiskit gates.

[ibrahim-shehzad]

To reproduce this error, I ran the following piece of code on the subscircuit "A" that was generated in this tutorial:

from qiskit import qpy 
with open('bell.qpy', 'wb') as fd: 
    qpy.dump(subcircuits["A"], fd)

[caleb-johnson]

from qiskit import qpy with open('bell.qpy', 'wb') as fd: qpy.dump(subcircuits["A"], fd)

It looks like serialization causes instructions to be decomposed, as this error is thrown in _define. We don't allow qpd gates to be decomposed if their basis_id isn't set because it isn't defined what it should decompose to.

We had suggested letting the gate decompose randomly according to its distribution, but we preferred to limit randomness in the code.

This becomes a problem when you want to run circuit knitting workloads remotely, because you'd like to serialize the QuantumCircuit. We need to find out if we can prohibit our QPDGates from decomposing during serialization somehow.

[garrison]

To reproduce this error

Thanks. FWIW, here's a full MWE:

from pathlib import Path
from tempfile import TemporaryDirectory

from qiskit.circuit.library import EfficientSU2
from qiskit import qpy

from circuit_knitting.cutting import partition_problem

qc = EfficientSU2(4, entanglement="linear", reps=2).decompose()
qc.assign_parameters([0.4] * len(qc.parameters), inplace=True)

partitioned_problem = partition_problem(circuit=qc, partition_labels="AABB")

with TemporaryDirectory() as tmpdir:
    with open(Path(tmpdir) / "bell.qpy", "wb") as fd:
        qpy.dump(partitioned_problem.subcircuits["A"], fd)

The full traceback is

Traceback (most recent call last):
  File "/home/garrison/serverless/417-mwe.py", line 17, in <module>
    qpy.dump(partitioned_problem.subcircuits["A"], fd)
  File "/home/garrison/serverless/.direnv/python-3.11.0/lib/python3.11/site-packages/qiskit/utils/deprecation.py", line 182, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/garrison/serverless/.direnv/python-3.11.0/lib/python3.11/site-packages/qiskit/qpy/interface.py", line 168, in dump
    writer(file_obj, program, metadata_serializer=metadata_serializer)
  File "/home/garrison/serverless/.direnv/python-3.11.0/lib/python3.11/site-packages/qiskit/qpy/binary_io/circuits.py", line 981, in write_circuit
    _write_custom_operation(
  File "/home/garrison/serverless/.direnv/python-3.11.0/lib/python3.11/site-packages/qiskit/qpy/binary_io/circuits.py", line 698, in _write_custom_operation
    elif operation.definition is not None:
         ^^^^^^^^^^^^^^^^^^^^
  File "/home/garrison/serverless/.direnv/python-3.11.0/lib/python3.11/site-packages/qiskit/circuit/instruction.py", line 239, in definition
    self._define()
  File "/home/garrison/serverless/circuit-knitting-toolbox2/circuit_knitting/cutting/qpd/instructions/qpd_gate.py", line 201, in _define
    raise ValueError(
ValueError: Missing 'basis_id': unable to realize SingleQubitQPDGate.

Specifically, the call to -- and definition of -- _write_custom_operation at https://github.com/Qiskit/qiskit/blob/main/qiskit/qpy/binary_io/circuits.py seem most relevant.

We had suggested letting the gate decompose randomly according to its distribution, but we preferred to limit randomness in the code.

That, and, crucially: this would be fine for TwoQubitQPDGate, but if you were to realize each SingleQubitQPDGate this way, you'd lose the covariance that must exist between the two halfs of the decomposition, so there would be no way to reconstruct correctly after taking the randomness locally in this way.

We need to find out if we can prohibit our QPDGates from decomposing during serialization somehow.

Agreed. :)

[garrison]

I don't know if this is a "proper" fix, but the following change to Qiskit would probably save us here. It is worth testing.

diff --git a/qiskit/qpy/binary_io/circuits.py b/qiskit/qpy/binary_io/circuits.py
index 2cecd7e31..3c98b8e66 100644
--- a/qiskit/qpy/binary_io/circuits.py
+++ b/qiskit/qpy/binary_io/circuits.py
@@ -740,7 +740,7 @@ def _write_custom_operation(file_obj, name, operation, custom_operations, use_sy
         num_ctrl_qubits = operation.num_ctrl_qubits
         ctrl_state = operation.ctrl_state
         base_gate = operation.base_gate
-    elif operation.definition is not None:
+    elif not operation._directive and operation.definition is not None:
         has_definition = True
         data = common.data_to_binary(operation.definition, write_circuit)
         size = len(data)

[garrison]

The above tweak worked. So does the following, which I feel may be closer to a proper fix:

diff --git a/qiskit/circuit/instruction.py b/qiskit/circuit/instruction.py
index 6b5ff71cf..0b2d69ff2 100644
--- a/qiskit/circuit/instruction.py
+++ b/qiskit/circuit/instruction.py
@@ -297,7 +297,7 @@ class Instruction(Operation):
     @property
     def definition(self):
         """Return definition in terms of other basic gates."""
-        if self._definition is None:
+        if self._definition is None and not self._directive:
             self._define()
         return self._definition

I'm running the Qiskit test suite now to see if this change causes any other fallout.

[garrison]

Another -- perhaps better -- way we could fix this is by making _define return immediately, just like its default implementation, rather than raise an error, when basis_id is None. This would also allow us to fix this without relying on upstream (Qiskit) to merge a change. I'm going to work on a PR.

[caleb-johnson]

Another -- perhaps better -- way we could fix this is by making _define return immediately, just like its default implementation, rather than raise an error, when basis_id is None. This would also allow us to fix this without relying on upstream (Qiskit) to merge a change. I'm going to work on a PR.

Oh, interesting. If that works, that's probably better than erroring anyway.

We also need to consider that a user can have a QPD circuit with its basis_ids set, serialize it, deserialize it, and now have a circuit which is realized with Qiskit gates. I'm not sure that is intended either. Maybe we should consider this as a whole?

[garrison]

We also need to consider that a user can have a QPD circuit with its basis_ids set, serialize it, deserialize it, and now have a circuit which is realized with Qiskit gates.

It almost seems like this might be what is intended by qpy. For instance, EfficientSU2 gets realized into its fundamental components as part of serializing it. But yes, TwoQubitQPDGate does feel a bit different. This is something we should ask around about to better understand.

Maybe we should consider this as a whole?

I opened #443 as an issue related to the current one, but yes, you raise another good point too, which is a bit independent of what we decide there (in #443).