-
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 backend.configuration() to use BackendConfiguration #1323
Merged
diego-plan9
merged 13 commits into
Qiskit:master
from
diego-plan9:feature/models-backend-configuration
Nov 26, 2018
Merged
Update backend.configuration() to use BackendConfiguration #1323
diego-plan9
merged 13 commits into
Qiskit:master
from
diego-plan9:feature/models-backend-configuration
Nov 26, 2018
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update `BaseBackend.configuratio()` in order to use the model object `BackendConfiguration`, removing a check from the init (as the object will be schema-conformant) and tweaking docstrings in the process.
Update the configuration of the rest of local simulators. Original credit by @ajavadia in Qiskit#1279.
Update the simulator configurations in order: * remove the `coupling_map` parameter, as it needs to be a list per the specs and had the value "all-to-all". * add the `gates` parameter, as an empty list (that will likely need to be populated once it is used anywhere) and it is a required parameter per the spec. Revise the discovery during Aer building, and adjust the checks done during instantiation to account for the usage of the object, removing obsolete code (since the object is guaranteed to have some keys such as backend_name).
Revise the methods in transpiler that accessed backend configuration, replacing the indexing by key with property accessing, and the treatment of basis_gates (str vs list). Adjust tests.
Update `configuration.gates` for the local simulators, as the specs require them to have at least one element. A temporary gate was added for the purposes of passing validation, which should be replaced with the real gates eventually.
Modify `IBMQBackend` in order to use `BackendConfiguration` for its `.configuration()` method, by: * moving the processing of the dict returned from the API to the `IBMQconnector` layer (cleanup case, add missing fields, handle specific fields). This changes has been verified against QX and IBMQ current returned values, and hopefully will be temporary until 0.7. * updating `IBMQProvider` instantiation and discovery (in the provider), removing outdated checks. * update `providerutils` to work with objects instead of dicts. Adjust tests accordingly, finally enabling the real functionality for `test_remote_backend_configuration`.
diego-plan9
requested review from
1ucian0,
ajavadia,
atilag,
delapuente,
ewinston,
jaygambetta,
mtreinish,
nonhermitian and
taalexander
as code owners
November 23, 2018 13:06
ajavadia
approved these changes
Nov 26, 2018
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.
Thanks @diego-plan9. I went through this and it looks good to merge to me.
@1ucian0 also ran the slow tests and everything seems to be working.
Thanks to you both! |
This was referenced Nov 26, 2018
lia-approves
pushed a commit
to edasgupta/qiskit-terra
that referenced
this pull request
Jul 30, 2019
* update configuration for statevector simulators * Update configuration() using BackendConfiguration Update `BaseBackend.configuratio()` in order to use the model object `BackendConfiguration`, removing a check from the init (as the object will be schema-conformant) and tweaking docstrings in the process. * Update default configuration for local simulators Update the configuration of the rest of local simulators. Original credit by @ajavadia in Qiskit#1279. * Updates to simulator configurations, aer Update the simulator configurations in order: * remove the `coupling_map` parameter, as it needs to be a list per the specs and had the value "all-to-all". * add the `gates` parameter, as an empty list (that will likely need to be populated once it is used anywhere) and it is a required parameter per the spec. Revise the discovery during Aer building, and adjust the checks done during instantiation to account for the usage of the object, removing obsolete code (since the object is guaranteed to have some keys such as backend_name). * Adjust transpiler access to configuration(), tests Revise the methods in transpiler that accessed backend configuration, replacing the indexing by key with property accessing, and the treatment of basis_gates (str vs list). Adjust tests. * Update configuration.gates with one gate Update `configuration.gates` for the local simulators, as the specs require them to have at least one element. A temporary gate was added for the purposes of passing validation, which should be replaced with the real gates eventually. * Update IBMQBackend to use BackendConfiguration Modify `IBMQBackend` in order to use `BackendConfiguration` for its `.configuration()` method, by: * moving the processing of the dict returned from the API to the `IBMQconnector` layer (cleanup case, add missing fields, handle specific fields). This changes has been verified against QX and IBMQ current returned values, and hopefully will be temporary until 0.7. * updating `IBMQProvider` instantiation and discovery (in the provider), removing outdated checks. * update `providerutils` to work with objects instead of dicts. Adjust tests accordingly, finally enabling the real functionality for `test_remote_backend_configuration`. * Fix unitary simulator name during rebasing * Update CHANGELOG
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issues:
#996 -
backend_configuration_schema.json
checkboxSummary
In a similar spirit to #1301, update
backend.configuration()
(base, aer, and ibmq) in order to return instances ofBaseConfiguration
that conform to the schemas instead of dicts. The method is also closely related to the instantiation of the backend (asconfiguration
is a parameter) and to the discovery made by theirProviders
, resulting in updates in those as well.For the local simulators, most of the changes in their configurations were handled by @ajavadia at #1279 (thanks - and sorry for the conflicts), and some adjustments were needed (more notably, adding
gates
and handling the simulatorscoupling_map
- see below).For
IBMQBackend
, the changes involved moving the parsing and processing of the configurations to theIBMQConnector
, freeing up the layer above from those modifications. Please note that the processing included is meant to be temporary, and takes into account the format currently returned by the API for all QX and IBMQ current devices and sims. Ideally, the call will be replaced with a call to a new endpoint that needs no preprocessing before the 0.7 release.Details and comments
There are some potential follow-ups for this PR:
gates
attribute that contains a single dummy gate (as the specs requires at least 1 element in them). The attribute is not really used by terra, but we should populate it properly with correct values.coupling_map
for the simulators has been removed, as its previous value (all-to-all
) is not valid according to the specs (they require a list). We should double-check the need for that attribute in the specs and the code (it is currently optional), and if needed come up with a default value (the PR assumes that if it is not present, it isNone
internally) tracing the transpiler logic (the changes included might give a hint).basis_gates
, the spec defines them as a list - which might enable us to finally get rid of passing them as a string during all the nested calls. The PR actually re-joins
them into a comma-separated string to preserve the behaviour and avoid more noise in this PR, but it might be a good time to finally address that.