Pulse backend configuration #2216
Conversation
taalexander
requested review from
1ucian0,
ajavadia,
atilag,
chriseclectic,
delapuente,
ewinston,
jaygambetta,
kdk,
mtreinish and
nonhermitian
as code owners
| @@ -69,6 +97,37 @@ class BackendConfigurationSchema(BaseSchema): | |||
| tags = List(String()) | |||
|
|
|||
|
|
|||
| class QASMBackendConfigurationSchema(BackendConfigurationSchema): | |||
There was a problem hiding this comment.
Can you rename the QASMFoo classes to QasmFoo (ie. QasmBackendConfigurationSchema), for consistency with the naming we use in the Qobj models (QasmQobj, etc)?
| Complex, Float, Dict, InstructionParameter) | ||
| from qiskit.validation.validate import PatternProperties | ||
|
|
||
| # pylint: disable=invalid-name,bad-super-call |
There was a problem hiding this comment.
Can you move the invalid-name to inside the specific classes or lines that need it? bad-super-call is also likely to go away with the from_dict changes.
| memory = Boolean(required=True) | ||
| max_shots = Integer(required=True, validate=Range(min=1)) | ||
|
|
||
| # Optional properties. | ||
| open_pulse = Boolean() |
There was a problem hiding this comment.
If both subclasses define open_pulse as required (and actually it is required and provided by the backends), can you leave it as required?
| self.memory = memory | ||
| self.max_shots = max_shots | ||
|
|
||
| super().__init__(**kwargs) | ||
|
|
||
| @classmethod | ||
| def from_dict(cls, dict_): |
There was a problem hiding this comment.
Can you remove the customized from_dict methods?
I think I see the intent, but the main advantage (not breaking existing code) should already be achieved by the subclassing, if done correctly (as in, if existing code instantiates a generic BackendConfiguration for a qasm backend in the same way as before, it would not break or actually change - specially if the change for open_pulse not required is reverted; and for pulse backends we have the chance of modifying them as they have not been published in a stable release yet). Removing from_dict would help keeping the models as just containers - I think we are better off not moving functionality into them: even if is a relatively small change from the line counts point of view, it has some implications at the arquitectural level that I'd rather avoid.
In turn, this probably means adjusting the individual backends for using the specific subclasses - can you include the changes for basicaer in this PR, and open an issue in the qiskit-ibmq-provider repo for handling those in that provider?
| @@ -0,0 +1,43 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
With the suggested change regarding from_dict, these tests might be rendered obsolete - instead, can you tune python.test_schemas.TestSchemaExamples#test_examples_are_valid so it uses the specific subclasses (should be a matter of updating obj_map and explicitly including backend_configuration_openpulse_example.json, as it is excluded by the if ad the end of the method)?
…er/qiskit-terra into issue-2116-pulse-backend-configuration
| memory (bool): backend supports memory. | ||
| max_shots (int): maximum number of shots supported. | ||
| """ | ||
|
|
||
| def __init__(self, backend_name, backend_version, n_qubits, basis_gates, | ||
| gates, local, simulator, conditional, open_pulse, memory, |
There was a problem hiding this comment.
Can you revert this line (and the docstring, plus line 125)?
* Add pulse backend configuration. * Add QASM and Pulse backend configuration objects. * Added QASMBackendConfiguration and PulseBackendConfiguration with factory method from_dict. * add tests for qasm/pulse backend configuration. * linting. * Parameter update * Cleanup provider models and rename. * linting * Linting * Make QASM -> Qasm * remove invalid name * Update open_pulse field to required. * Remove from_dict hackery. * Update tests. * linting. * linting. * linting and required variable. * readd comment.
Summary
This PR adds a pulse backend configuration (as well as a qasm backend configuration).
BackendConfigurationis now a factory method that decides on whether to initialize aQASMBackendConfigurationorBackendConfigurationbased on the value of the fieldopen_pulse.Closes #2116.
Details and comments
In particular, I would like an opinion on the implementation of
from_dictinBackendConfigurationthe main advantage of this approach is it does not break any existing code.