-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for saving/loading calibration parameters without schedules #1357
Add support for saving/loading calibration parameters without schedules #1357
Conversation
d337439
to
d92932c
Compare
I force pushed some lint fixes to keep everything important in this PR in the last commit (now d92932c). |
5b6fa06 added support for saving and loading `Calibrations` objects with a JSON format that preserved calibrated gate schedules. However, it did not capture and restore the `Parameter` objects for calibration parameters (like `drive_freq`) which were not associated with a schedule. This commit adds support for these parameters by adding an entry to the serialization model that holds a placeholder `QuantumCircuit` with the parameters attached to placeholder instructions. `ExperimentEncoder` can serialize `QuantumCircuit` to qpy and so supports this format. Using a placeholder circuit like this is the only supported way to serialize `Parameter` objects. An alternative approach would be to store and retrieve the hidden attributes of the `Parameter` objects directly.
d92932c
to
a9770c9
Compare
#1351 was merged and this has been rebased on it. |
(max(k.qubits or (0,)) for k in cals._parameter_map if k.schedule is None), | ||
default=0, | ||
) | ||
schedule_free_parameters = QuantumCircuit(max_qubit + 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this is a hack. Qpy also saves instruction and QuantumCircuit metadata, and using QuantumCircuit format will increase output data size and may cause edge case in future Qpy module updates. Although QuantumCircuit provides effective representation of parameters tied to particular qubits, I would prefer directly saving the parameter object as you wrote as a second approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this is a hack.
Yes, it is a hack, but there is no supported way to directly save a Parameter
object, so I think any method will be a bit of a hack.
Qpy also saves instruction and QuantumCircuit metadata, and using QuantumCircuit format will increase output data size
In practice, I don't think the overhead is too large for qpy since it tries to be efficient and no extra metadata is being added to the circuit or instructions in this code. I was curious what the comparison really was so I did a test. Here is the code that I used:
import json
import zlib
from io import BytesIO
from qiskit import QuantumCircuit
from qiskit.circuit import Instruction, Parameter
from qiskit.qpy import load, dump
from qiskit_experiments.framework.json import ExperimentEncoder
num_params = 100
qc = QuantumCircuit(1)
for idx in range(num_params):
qc.append(Instruction("drive_freq", 0, 0, [Parameter(f"param{idx}")]))
param_list = []
for idx in range(num_params):
param = Parameter("a")
param_list.append({"qubits": [0], "parameter_name": f"param{idx}", "uuid": param._uuid.bytes})
qpy_file = BytesIO()
pos = dump(qc, qpy_file)
qpy_file.seek(0)
new_circuit = load(qpy_file)[0]
qpy_file.seek(0)
json_bytes = json.dumps(param_list, cls=ExperimentEncoder).encode("utf-8")
json_bytes_zip = zlib.compress(json_bytes)
qpy_bytes = qpy_file.read()
print("number of parameters: ", num_params)
print("qpy bytes: ", len(qpy_bytes))
print("zipped qpy bytes: ", len(zlib.compress(qpy_bytes)))
print("json bytes: ", len(json_bytes))
print("zipped json bytes: ", len(json_bytes_zip))
and the results I got were:
number of parameters: 100
qpy bytes: 7859
zipped qpy bytes: 2214
json bytes: 16574
zipped json bytes: 2954
and
number of parameters: 200
qpy bytes: 15659
zipped qpy bytes: 4300
json bytes: 33374
zipped json bytes: 5790
So without compression, the qpy is about half as big as a JSON output. With compression, they are closer though qpy is still smaller.
may cause edge case in future Qpy module updates
My concern was that not using qpy would be more vulnerable to a future Qiskit update. Qiskit tries to keep backwards compatibility with qpy, maintaining loading of old qpy files in newer versions of Qiskit. That is more guarantee than we have that saving {"name": param._name, "uuid": param._uuid.bytes.encode("utf-8")}
will always robustly serialize a Parameter
.
Although QuantumCircuit provides effective representation of parameters tied to particular qubits, I would prefer directly saving the parameter object as you wrote as a second approach.
So would you prefer the approach I used in my test code, like the following?
schedule_free_parameters = [
{"qubits": list(s.qubits), "parameter_name": p.name, "uuid": p._uuid.bytes}
for s, p in cals._parameter_map.items()
if s.schedule is None
]
I still consider it a bit of a hack because it relies on Parameter._uuid
which is not a public attribute and is kind of an implementation detail. On the other hand, Parameter.__init__
does list uuid
as a parameter (for "advanced usage") so it is not fully private.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our Json Encoder does serialize Parameter object :) https://github.com/Qiskit-Extensions/qiskit-experiments/blob/0ee488989b67d0239597741bf2bb3e51b5d68965/qiskit_experiments/framework/json.py#L516-L521
Note that Parameter
is a subclass of ParameterExpression
. Since this uses the protected function (this code is also used by IBM's provider), the API stability is not really guaranteed, but at least data structure must be preserved in future version through QPY.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder what the use case was for adding ParameterExpression
to the encoder. In any case, it does not work currently:
and this is the kind of thing that I would like to avoid by using a public interface. So what is your preference for how to save the parameters. The options I see are:
- Save them in
QuantumCircuit
- Save the name and uuid directly
- Fix the serializer's use of
_write_parameter_expression
. Note that I think is kind of difficult. I looked into it and I needed to change_read_parameter_expression
to_read_parameter_expression_v3
which takes two extra arguments. That would only read new parameters. Old ones would need the old_read_parameter_expression
but the logic to choose between the two is in other qpy code. Also, note that this serializes aParameter
into aParameterExpression
so thesave_utils
code would need to pull theParameter
out of theParameterExpression
which starts to feel like pulling theParameter
out of the quantum circuit instructions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked how IBM Runtime does this serialization, and found that they implemented a custom logic in their JSON decoder. This means current implementation of the ExperimentDecoder is not correct. I agree using the protected method increases maintenance cost like this, and I'm fine with using a circuit as a container, considering our bandwidth for maintenance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, sounds good. Did you have fairly equal feeling about serializing quantum circuit versus name and uuid? I had doubts about using qpy serialization on Parameter directly but I was uncertain about the other two options. I made uuid a public property of Parameter in Qiskit if we wanted to go with serializing name and uuid directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's another option. But I feel this is not enough robust to future update of the Parameter class constructor. Qpy should guarantee the backward compatibility which makes our maintenance workload minimum :)
5b6fa06 added support for saving and
loading
Calibrations
objects with a JSON format that preservedcalibrated gate schedules. However, it did not capture and restore the
Parameter
objects for calibration parameters (likedrive_freq
) whichwere not associated with a schedule. This commit adds support for these
parameters by adding an entry to the serialization model that holds a
placeholder
QuantumCircuit
with the parameters attached to placeholderinstructions.
ExperimentEncoder
can serializeQuantumCircuit
to qpyand so supports this format. Using a placeholder circuit like this is
the only supported way to serialize
Parameter
objects. An alternativeapproach would be to store and retrieve the hidden attributes of the
Parameter
objects directly.Closes #1355