-
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
Update the qobj schema to support pulse gate calibrations. #4761
Conversation
…, which needed to be fixed in both the schema and the py files
…g other metadata in the future. Make a qobj.common file to allow qobj.qasm to use features of qobj.pulse
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 think this looks good except for needing top-level calibrations and my one comment regarding the lazy loading
09a454e
to
d75ecac
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.
Nitpicks
Co-authored-by: SooluThomas <[email protected]>
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 except for one backwards compatibility concern around moving the validator module property. I know that's being used downstream by people and we shouldn't break them unless we have to. We either should alias and deprecate it with a release note or just leave it in qobj/qasm.py
.
…ra into pulse-gates/schema
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.
LGTM, thanks for checking the deepcopy/pickling
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.
Looks good, all suggestions are non-blocking.
@@ -464,6 +476,19 @@ def __init__(self, qobj_id=None, config=None, experiments=None, | |||
self.type = 'QASM' | |||
self.schema_version = '1.3.0' | |||
|
|||
def _validate_json_schema(self, out_dict): | |||
class QobjEncoder(json.JSONEncoder): |
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.
Should this be broken out as a separate utility class?
], | ||
"instructions": [ | ||
{ | ||
"name": "1c5a0346ff72c04c45d536d50d13b0d42cf65dc8e320d32385d36861d3bb1e41", |
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.
These hashed names are pretty cryptic for an example.
""" | ||
Initialize a single gate calibration. Instructions may reference waveforms which should be |
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.
""" | |
Initialize a single gate calibration. Instructions may reference waveforms which should be | |
"""Initialize a single gate calibration. Instructions may reference waveforms which should be |
"type": "object", | ||
"properties": { | ||
"gates": { | ||
"description": "A list of dicts which contain all the required information to lower a gate to waveforms.", |
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.
"description": "A list of dicts which contain all the required information to lower a gate to waveforms.", | |
"description": "A list of objects which contain all the required information to lower a gate to waveforms.", |
"type": "object", | ||
"properties": { | ||
"gates": { | ||
"description": "A list of dicts which contain all the required information to lower a gate to waveforms.", |
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.
"description": "A list of dicts which contain all the required information to lower a gate to waveforms.", | |
"description": "A list of objects which contain all the required information to lower a gate to waveforms.", |
Summary
Begin Pulse Gates feature by updating the schema. Calibrations may be in the job
config
or the experimentconfig
.Example:
The
'gates'
information must be sufficient to match against any QASM Qobj instruction within thejob.experiments[i].instructions
fields, for instance:QasmQobjInstruction(name='rxtheta', params=[1.57], qubits=[1])
matches the last entry.We can move
pulse_library
to the top level to avoid duplicating memory-intensive pulse waveforms in a single job. The assembler will assert that each waveform in the pulse library is referenced uniquely.Details and comments
pulse_library
to qobj levelqobj.config.calibrations
fieldLimitations
The current iteration supports any possible job, however, it is of course less efficient than a parameterized Qobj would be.
This schema cannot support unbound parameters, which is also true for any circuit at the moment, except here: Qiskit/qiskit-aer#485. Supporting unbound parameters should be done in a follow up (e.g., a gate's params are not fully specified and a value in the
openpulse_instructions
depends on that param). This follow up should be useable/compatible with Pulse Schedule, as well.