-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
QPY schedule serialization #7300
QPY schedule serialization #7300
Conversation
2ed0a9a
to
f32989c
Compare
Pull Request Test Coverage Report for Build 2545753902
💛 - Coveralls |
adc2e58
to
2033738
Compare
qiskit/qpy/objects/schedules.py
Outdated
|
||
def _write_measure_processor(file_obj, data): | ||
name = data.name.encode("utf8") | ||
params = json.dumps(data.params, separators=(",", ":")).encode("utf8") |
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 assume parameters of Kernel
and Discriminator
are a free-form but json serializable dictionary (i.e. no numpy, no parameter, etc...). However I've never seen actual spec of these objects. @taalexander do you have any information?
c263532
to
21bfe1b
Compare
qiskit/qpy/objects/circuits.py
Outdated
|
||
def _parse_custom_instruction(custom_instructions, gate_name, params): | ||
(type_str, num_qubits, num_clbits, definition) = custom_instructions[gate_name] | ||
if type_str == "i": |
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.
TypeKey
enum strategy doesn't work here because i
is also used for Integer (because key is not scoped). This logic imperfection looks bit awkward (but removing enum makes pulse qpy code really hard to read because there are many kinds of operand classes).
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 if we want to have a custom instruction table type key enum defined in this module too? They're circuit specific but likely will grow more over time as we have special custom circuit operations with different representations that are needed. For example QPY v3 added a "p"
here for PauliEvolutionGate
da0715b
to
1f836c4
Compare
1f836c4
to
7c9f829
Compare
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.
This LGTM, thanks for the quick fixes. I left a few questions inline about the size of certain fields. I don't think it's a blocker at all (I'm going to tag as automerge) the choices you made are reasonable but I wanted to just raise the question so we have it in the review log, primarily because if we do need to bump any we'll have to bump the QPY version.
qiskit/qpy/formats.py
Outdated
@@ -155,8 +154,49 @@ | |||
CUSTOM_CIRCUIT_INST_DEF_PACK = "!H1cII?Q" | |||
CUSTOM_CIRCUIT_INST_DEF_SIZE = struct.calcsize(CUSTOM_CIRCUIT_INST_DEF_PACK) | |||
|
|||
# CALIBRATION | |||
CALIBRATION = namedtuple("CALIBRATION", ["num_cals"]) | |||
CALIBRATION_PACK = "!I" |
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'm not sure how many calibrations are typical on a circuit, this sets the upper bound at 2**32
which is probably sufficient but I'm wondering if we want to allow more or less.
qiskit/qpy/formats.py
Outdated
|
||
# CALIBRATION_DEF | ||
CALIBRATION_DEF = namedtuple("CALIBRATION_DEF", ["name_size", "num_qubits", "num_params", "type"]) | ||
CALIBRATION_DEF_PACK = "!III1c" |
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.
Same question here as above we're limiting the calibration name, number of qubits, and number of parameters to 2**32. I think that's reasonable but maybe too high.
"amp_limited", | ||
], | ||
) | ||
SYMBOLIC_PULSE_PACK = "!HHHH?" |
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.
This limits the number of bytes to the envelope, constraints, and valid amp conditions size to 2**16 bytes. I know they're zlib compressed but do you have as sense for how large these can get?
|
||
# MAP_ITEM | ||
MAP_ITEM = namedtuple("MAP_ITEM", ["key_size", "type", "size"]) | ||
MAP_ITEM_PACK = "!H1cH" |
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.
Do you think we can have keys more than 2*16 characters?
The support for serializing SymbolicPulse objects using QPY was added in PR Qiskit#7300 and has been released as part of 0.21.0. However, the documentation for the SymbolicPulse class had a note saying the QPY support was pending. This was necessary when the SymbolicPulse class was added in Qiskit#7821 because it was unclear when we'd be able to add the QPY support. But, Qiskit#7300 was updated and merged soon after Qiskit#7821, but we neglected to remove that note. This commit corrects the oversight and removes the stale note.
The support for serializing SymbolicPulse objects using QPY was added in PR #7300 and has been released as part of 0.21.0. However, the documentation for the SymbolicPulse class had a note saying the QPY support was pending. This was necessary when the SymbolicPulse class was added in #7821 because it was unclear when we'd be able to add the QPY support. But, #7300 was updated and merged soon after #7821, but we neglected to remove that note. This commit corrects the oversight and removes the stale note. Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
The support for serializing SymbolicPulse objects using QPY was added in PR #7300 and has been released as part of 0.21.0. However, the documentation for the SymbolicPulse class had a note saying the QPY support was pending. This was necessary when the SymbolicPulse class was added in #7821 because it was unclear when we'd be able to add the QPY support. But, #7300 was updated and merged soon after #7821, but we neglected to remove that note. This commit corrects the oversight and removes the stale note. Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 47c7c27)
…#8292) The support for serializing SymbolicPulse objects using QPY was added in PR #7300 and has been released as part of 0.21.0. However, the documentation for the SymbolicPulse class had a note saying the QPY support was pending. This was necessary when the SymbolicPulse class was added in #7821 because it was unclear when we'd be able to add the QPY support. But, #7300 was updated and merged soon after #7821, but we neglected to remove that note. This commit corrects the oversight and removes the stale note. Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 47c7c27) Co-authored-by: Matthew Treinish <[email protected]>
Co-authored-by: Matthew Treinish <[email protected]> Co-authored-by: Naoki Kanazawa <[email protected]>
* Copy Qiskit/qiskit#8055 Co-authored-by: Matthew Treinish <[email protected]> * Copy qpy changes from Qiskit/qiskit#8093 Co-authored-by: Jake Lishman <[email protected]> Co-authored-by: Jake Lishman <[email protected]> * Copy Qiskit/qiskit#8200 Co-authored-by: Matthew Treinish <[email protected]> * Copy of Qiskit/qiskit#7300 Co-authored-by: Matthew Treinish <[email protected]> Co-authored-by: Naoki Kanazawa <[email protected]> * Copy of Qiskit/qiskit#8235 Co-authored-by: Matthew Treinish <[email protected]> Co-authored-by: Matthew Treinish <[email protected]> Co-authored-by: Jake Lishman <[email protected]> Co-authored-by: Jake Lishman <[email protected]> Co-authored-by: Naoki Kanazawa <[email protected]>
Summary
This PR adds
qiskit.qpy
that covers qpy serialization of bothQuantumCircuit
andScheduleBlock
which is requirement of Qiskit Experiments.Details and comments
Currently only basic component for MVP
Because in pulse schedule representation (in contrast to
QuantumCircuit
), parameterizable operand values exit across multiple layers from alignment context parameter to pulse values. To eliminate boilerplate code in each layer, parameter value serialization logic is isolated without changing data structure so that it can be reused.TODO
[ ] Call serialization[ ] Discriminator/Kernel serializationQuantumCircuit.calibrations
field serialization will be done in follow-up.