Skip to content

Add support for saving/loading calibration parameters without schedules #1357

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 31, 2024
Merged

Add support for saving/loading calibration parameters without schedules #1357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a9770c9
Add support for saving/loading calibration parameters without schedules
wshanks Jan 16, 2024
7374d97
Merge branch 'main' into save-cals-no-schedule-params
wshanks Jan 20, 2024
37985d4
Merge branch 'main' into save-cals-no-schedule-params
wshanks Jan 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions qiskit_experiments/calibration_management/save_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
from datetime import datetime
from typing import List, Dict, Any

from qiskit import QuantumCircuit
from qiskit.circuit import Instruction
from qiskit.pulse import ScheduleBlock

from .calibrations import Calibrations
Expand Down Expand Up @@ -118,6 +120,14 @@ class CalibrationModelV1:
parameters: List[ParameterModelV1] = field(default_factory=list)
"""List of calibrated pulse parameters."""

schedule_free_parameters: QuantumCircuit = field(default_factory=lambda: QuantumCircuit(1))
"""Placeholder circuit for parameters not associated with a schedule

The circuit contains placeholder instructions which have the Parameter
objects attached and operate on the qubits that the parameter is associated
with in the calibrations.
"""

schema_version: str = "1.0"
"""Version of this data model. This must be static."""

Expand Down Expand Up @@ -177,13 +187,26 @@ def calibrations_to_dict(
sched_obj.metadata.update(qubit_metadata)
sched_entries.append(sched_obj)

max_qubit = max(
(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)
Copy link
Collaborator

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.

Copy link
Collaborator Author

@wshanks wshanks Jan 20, 2024 •
edited
Loading

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.

Copy link
Collaborator

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.

Copy link
Collaborator Author

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:

image

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:

  1. Save them in QuantumCircuit
  2. Save the name and uuid directly
  3. 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 a Parameter into a ParameterExpression so the save_utils code would need to pull the Parameter out of the ParameterExpression which starts to feel like pulling the Parameter out of the quantum circuit instructions.

Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator

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 :)

for sched_key, param in cals._parameter_map.items():
if sched_key.schedule is None:
schedule_free_parameters.append(
Instruction("parameter_container", len(sched_key.qubits), 0, [param]),
sched_key.qubits,
)

model = CalibrationModelV1(
backend_name=cals.backend_name,
backend_version=cals.backend_version,
device_coupling_graph=getattr(cals, "_coupling_map"),
control_channel_map=ControlChannelMap(getattr(cals, "_control_channel_map")),
schedules=sched_entries,
parameters=data_entries,
schedule_free_parameters=schedule_free_parameters,
)

return asdict(model)
Expand Down Expand Up @@ -257,6 +280,15 @@ def calibrations_from_dict(
schedule=param.schedule,
update_inst_map=False,
)

for instruction in model.schedule_free_parameters.data:
# For some reason, pylint thinks the items in data are tuples instead
# of CircuitInstruction. Remove the following line if it ever stops
# thinking that:
# pylint: disable=no-member
for param in instruction.operation.params:
cals._register_parameter(param, instruction.qubits)

cals.update_inst_map()

return cals
4 changes: 0 additions & 4 deletions test/calibration/test_calibrations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

from test.base import QiskitExperimentsTestCase
import os
import unittest
import uuid
from collections import defaultdict
from datetime import datetime, timezone, timedelta
Expand Down Expand Up @@ -1692,9 +1691,6 @@ def test_save_load_library_csv(self):
BackendData(backend).drive_freqs[0],
)

# Expected to fail because json calibration loading does not support
# restoring Parameter objects
@unittest.expectedFailure
def test_save_load_library(self):
"""Test that we can load and save a library.

Expand Down