Skip to content
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

Saving and loading calibrations data #1120

Merged

Conversation

nkanazawa1989
Copy link
Collaborator

Summary

This PR adds support for full roundtrip of Calibrations instance through JSON file format. Even though this PR is blocked by the following PR, this branch is ready to streamline calibration tasks.

This is blocked by Qiskit/qiskit#9890

Details and comments

  • Update JSON Encoder and Decoder to support ScheduleBlock and datetime object
  • Add save utils that provides a standard data model for calibration. Note that this offers portability of calibration data across different versions, platforms, etc..
  • Deprecate conventional csv format that only supports calibration table and bit complicated.

Sample code for testing

Checkout my Terra folk https://github.com/nkanazawa1989/qiskit-terra/tree/feature/qpy-schedule-reference

from qiskit_experiments.calibration_management import Calibrations
from qiskit import pulse
from qiskit.circuit import Parameter

cals = Calibrations(
    coupling_map=[[0, 1]],
    control_channel_map={(0, 1): [pulse.ControlChannel(0)], (1, 0): [pulse.ControlChannel(1)]},
)

with pulse.build(name="my_gate") as sched:
    pulse.play(
        pulse.Constant(Parameter("duration"), Parameter("amp")),
        pulse.ControlChannel(Parameter("ch0.1")),
    )

cals.add_schedule(sched, qubits=(0, 1))
cals.add_parameter_value(100, "duration", qubits=(0, 1), schedule="my_gate")
cals.add_parameter_value(0.123, "amp", qubits=(0, 1), schedule="my_gate")

cals.save(file_prefix="data", overwrite=True)
roundtrip = Calibrations.load("./data.json")

sched = cals.get_schedule("my_gate", (0, 1))
roundtrip_sched = roundtrip.get_schedule("my_gate", (0, 1))

assert sched == roundtrip_sched

Copy link
Contributor

@eggerdj eggerdj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this! Have you tried checking if the links between parameters are preserved after a round trip serialize/deserialize? Can you add a few tests for that?

qiskit_experiments/calibration_management/calibrations.py Outdated Show resolved Hide resolved
Comment on lines +1378 to +1380
file_path = file_prefix + ".json"
if os.path.isfile(file_path) and not overwrite:
raise CalibrationError(f"{file_path} already exists. Set overwrite to True.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test to check that we raise if overwrite is False and there already exists a file with this name?

qiskit_experiments/calibration_management/save_utils.py Outdated Show resolved Hide resolved
qiskit_experiments/calibration_management/save_utils.py Outdated Show resolved Hide resolved
qiskit_experiments/calibration_management/save_utils.py Outdated Show resolved Hide resolved
Comment on lines 172 to 177
for sched in model.schedules:
cals.add_schedule(
schedule=sched.data,
qubits=sched.qubits if len(sched.qubits) != 0 else None,
num_qubits=sched.num_qubits,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this keep any parameter links that we initially had?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. QPY preserves parameter UUID and it serializes the schedule through QPY format.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok good. Thanks, do we have a test on this?

qiskit_experiments/calibration_management/save_utils.py Outdated Show resolved Hide resolved
nkanazawa1989 and others added 4 commits April 17, 2023 13:20
- Docs update
- Remove schema for schedule and move qubit info to metadata
- Change function names

Co-authored-by: Daniel Egger <egger.d.j@gmail.com>
- Fix Calibrations eq logic. This logic uses json dump for dict serialization which doesn't work for complex parameter value.
- Fix test without deprecation. Revert logic change to Calibrations.config method which is no longer used anyways.
- Fix wrong import
eggerdj added a commit that referenced this pull request May 9, 2023
### Summary
This PR aims to remove the use of complex amplitude for `SymbolicPulse`
in calibration experiments. Most notable changes are to the
`HalfAngleCal` experiment and the `FixedFrquencyTransmon` library. With
these changes, support of complex values in general raises a
`PendingDeprecationWarning`.

### Details and comments
Qiskit Terra recently changed the representation of `SymbolicPulse` from
complex amplitude to (`amp`,`angle`). Once the deprecation is completed,
some calibration experiments will fail. Additionally, assignment of
complex parameters in general has caused problems recently (See
Qiskit-Terra issue
[9187](Qiskit/qiskit#9187)), and is also
being phased out (See Qiskit-Terra PR
[9735](Qiskit/qiskit#9735)).

Most calibration experiments are oblivious to these changes, with the
exception of `HalfAngleCal` and `RoughAmplitudeCal`. The library
`FixedFrequencyTransmon` also has to conform with the new
representation.

To create as little breaking changes as possible, the following were
changed:

- `FixedFrequencyTransmon` library was converted to the new
representation. All experiments will work as they have been with it.
- `RoughAmplitudeCal` was changed such that it will work for both real
or complex `amp`, without changing the type of the value.
- `HalfAngleCal` was changed to calibrate 'angle' instead of the complex
amplitude. A user which uses the `FixedFrequencyTransmon` library will
experience no change (except for the added parameters). A user which
uses custom built schedules will experience an error. To simplify the
transition, most likely scenarios (schedule with no `angle` parameter,
`cal_parameter_name="amp"`) will raise an informative error with
explanation about the needed changes.

A `PendingDeprecationWarning` is raised with every initialization of
`ParameterValue` with complex type value (which also covers addition of
parameter value to a calibration). Note that Qiskit-Terra PR
[9897](Qiskit/qiskit#9897) will also raise
a warning from the Terra side, for all assignment of complex parameters.

Handling of loaded calibrations which do not conform to the new
representation will be sorted out once PR #1120 is merged, as it
introduces a major change in calibration loading.

---------

Co-authored-by: Naoki Kanazawa <nkanazawa1989@gmail.com>
Co-authored-by: Will Shanks <wshaos@posteo.net>
Co-authored-by: Daniel J. Egger <38065505+eggerdj@users.noreply.github.com>
@nkanazawa1989 nkanazawa1989 added this to the Release 0.6 milestone May 12, 2023
Copy link
Contributor

@eggerdj eggerdj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is good. I have a few comments mainly related to the messaging.

qiskit_experiments/calibration_management/calibrations.py Outdated Show resolved Hide resolved
qiskit_experiments/calibration_management/calibrations.py Outdated Show resolved Hide resolved
qiskit_experiments/calibration_management/calibrations.py Outdated Show resolved Hide resolved
qiskit_experiments/calibration_management/calibrations.py Outdated Show resolved Hide resolved
qiskit_experiments/calibration_management/save_utils.py Outdated Show resolved Hide resolved
qiskit_experiments/calibration_management/save_utils.py Outdated Show resolved Hide resolved
qiskit_experiments/calibration_management/save_utils.py Outdated Show resolved Hide resolved
test/calibration/test_calibrations.py Show resolved Hide resolved
nkanazawa1989 and others added 3 commits May 15, 2023 13:21
Co-authored-by: Daniel J. Egger <38065505+eggerdj@users.noreply.github.com>
Copy link
Contributor

@eggerdj eggerdj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@nkanazawa1989 nkanazawa1989 added this pull request to the merge queue Jun 2, 2023
Merged via the queue into Qiskit-Extensions:main with commit 5b6fa06 Jun 2, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants