Closed adekusar-drl closed 8 months ago
This seems to be an issue with the qpy serialization/deserialization.
This example constructs the same qc
provided above in the original description of the issue, prints it, then serializes and deserializes it, and prints it again. The original circuit qc
differs from new circuit new_qc
: in the new circuit the param u[1]
does not appear.
from qiskit import QuantumCircuit
from qiskit.circuit import ParameterVector
def create_layer(num_qubits, params):
qc = QuantumCircuit(num_qubits, name="L")
param_index = 0
for qubit1 in range(0, num_qubits - 1, 2):
qubit2 = qubit1 + 1
unit_params = params[param_index: param_index + 1]
add_unit(qc, qubit1, qubit2, unit_params)
param_index += 1
return qc.to_instruction()
def add_unit(qc: QuantumCircuit, qubit1, qubit2, params):
u = QuantumCircuit(2, name="U")
u.rz(params[0], 0)
qc.compose(u.to_instruction(), [qubit1, qubit2], inplace=True)
num_qubits = 4
qc = QuantumCircuit(num_qubits)
num_params = 2
params = ParameterVector("u", num_params)
layer = create_layer(num_qubits, params)
qc.compose(layer, list(range(num_qubits)), inplace=True)
# print original circuit
print(qc.decompose(reps=2))
# convert to qpy and back to QuantumCircuit
from qiskit import qpy
with open('temp.qpy', 'wb') as fd:
qpy.dump(qc, fd)
with open('temp.qpy', 'rb') as fd:
new_qc = qpy.load(fd)[0]
# print new circuit
print(new_qc.decompose(reps=2))
Output: qc:
┌──────────┐
q_0: ┤ Rz(u[0]) ├
└──────────┘
q_1: ────────────
┌──────────┐
q_2: ┤ Rz(u[1]) ├
└──────────┘
q_3: ────────────
new_qc:
┌──────────┐
q_0: ┤ Rz(u[0]) ├
└──────────┘
q_1: ────────────
┌──────────┐
q_2: ┤ Rz(u[0]) ├
└──────────┘
q_3: ────────────
Hi. @mberna In order to update the client, is there any news on that issue?
Hi @fvarchon , this case actually hits a limitation of qpy
that has an easy fix on the user side.
qpy
relies on the operation name for serializing/deserializing. This allows it to only store the definition for custom gates once and then re-use it for all instances of the gate. In this examples, the add_unit
method is hard-coding the name "U"
on two units that are actually different, as one is parametrized with u[0]
, and the other one with u[1]
. There is no way for qpy to currently check if the first "U"
is different to the second "U"
, and this is why they are both deserialized as the first "definition" of "U"
. Making sure both layers have different names (u = QuantumCircuit(2, name=f"U_{id}")
) will lead to the correct serialization:
from qiskit import QuantumCircuit
from qiskit.circuit import ParameterVector
def create_layer(num_qubits, params):
qc = QuantumCircuit(num_qubits, name="L")
param_index = 0
for qubit1 in range(0, num_qubits - 1, 2):
qubit2 = qubit1 + 1
unit_params = params[param_index: param_index + 1]
add_unit(qc, qubit1, qubit2, unit_params, qubit1)
param_index += 1
return qc.to_instruction()
def add_unit(qc: QuantumCircuit, qubit1, qubit2, params, id):
u = QuantumCircuit(2, name=f"U_{id}")
u.rz(params[0], 0)
qc.compose(u.to_instruction(), [qubit1, qubit2], inplace=True)
num_qubits = 4
qc = QuantumCircuit(num_qubits)
num_params = 2
params = ParameterVector("u", num_params)
layer = create_layer(num_qubits, params)
qc.compose(layer, list(range(num_qubits)), inplace=True)
# print original circuit
print(qc.decompose(reps=2))
# convert to qpy and back to QuantumCircuit
from qiskit import qpy
with open('temp.qpy', 'wb') as fd:
qpy.dump(qc, fd)
with open('temp.qpy', 'rb') as fd:
new_qc = qpy.load(fd)[0]
# print new circuit
print(new_qc.decompose(reps=2))
Output: qc:
┌──────────┐
q_0: ┤ Rz(u[0]) ├
└──────────┘
q_1: ────────────
┌──────────┐
q_2: ┤ Rz(u[1]) ├
└──────────┘
q_3: ────────────
qc2:
┌──────────┐
q_0: ┤ Rz(u[0]) ├
└──────────┘
q_1: ────────────
┌──────────┐
q_2: ┤ Rz(u[1]) ├
└──────────┘
q_3: ────────────
While I agree that using the gate name is clearly not "bullet-proof", it's the mechanism we currently have, and modifying it could have memory or runtime costs that need to be considered carefully. However, as you can see, it's easy to fix once the issue is known. Do you think this could be enough to help the client? (and @adekusar-drl)
@ElePT thanks for the update, I overcame the issue long time ago, honestly. I realized that it is better not use nested circuits/instructions and so on despite the fact they are very handy for complex circuits. But the issue is really annoying. The proposed solution is okay, but nevertheless the issue to be fixed. It is very common to debug on reference primitives and then move to cloud. And then fixing the issue becomes crucial when you queued for ages and then you get a nasty error in your circuit.
Closing since this is a duplicate of https://github.com/Qiskit/qiskit/issues/8941
Describe the bug Runtime Estimator fails when a complex circuit that is made of instructions created from other circuit is executed. The same circuit runs perfectly well on the reference primitives locally. The script below reproduces the problem. It may look artificial, but a lot of code has been removed to make the script shorter. I do believe the same problem will occur with Sampler.
Steps to reproduce Add a
TOKEN
definition to the script and run it.Exception is raised
qiskit.circuit.exceptions.CircuitError: \'Cannot bind parameters (u[1]) not present in the circuit.\'
Full log:
Expected behavior No exception is thrown
Suggested solutions There's a workaround. Replace
cloud_execute(qc)
withcloud_execute(qc.decompose(reps=3)
and the problem is solved, but it is inconvenient.Additional Information