Use validated models for Result, update public API #1360
Merged
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
Replace the existing `qiskit.Result` with the model-based class, previously in `qiskit.validation.result`, splitting it in two files for convenience. Not all functionality has been moved to the new file yet. Remove the utilities in file_io, as they are superseeded by the direct conversion of results objects from and to json.
Complete the replacing of `qiskit.Result` with the model-based class: * move the remaining old methods of `_result` to the new class, with extra comments for deciding on the ones that make the public API. * remove `qobj/_result.py` and `result/_result.py`. * revise `result_from_old_style_dict`
Update the backends for the new-style results: * revise usage of `result_from_old_style_dict`, dropping the second parameter. * adjust statevector handling of snapshots. * adjust unitary creation of the vector, converting complex numbers to pairs.
Update a number of tests for the new style results: * use a temporary function for converting between count keys as binary to hex keys. * skipping a number of tests (tomography, result aggregation functions) marking them as follow-ups. * misc adjustments in a lot of places.
Include `bin_to_hex` as part of `assertDictAlmostEqual`, in order to reduce the number of adjustments needed for the tests.
Update the `qiskit.backends.ibmq` module in order to deal with new style results, adjusting tests accordingly.
Update the backends and the helper functions to ensure that the circuit name is available as `result.results[x].header.name`. Update examples accordingly.
Also get_circuit_status, get_job_id.
Deprecated `get_names()` and revise the docstrings of the non deprecated functions.
Remove the `__len__` and `__getitem__` methods of Results, in turn removing checking for length and indexing. Those methods were not used in the tests (except in a couple of occurences), and deprecating was not as clean as with regular methods - this can be revised if needed. Lint and style tweaks.
Removing again the "removed" duplicated section, adding entries.
diego-plan9
requested review from
1ucian0,
ajavadia,
atilag,
delapuente,
ewinston,
jaygambetta,
mtreinish,
nonhermitian and
taalexander
as code owners
diego-plan9
changed the title
|
going to approve but there needs to be a follow up that makes the get_counts() process the data and return it in the standard format. We will also rename get_data just data but keep the other get_counts, get_unitary, get_snapshots and get_statevector as get to imply that we are doing some postprocessing not just returning the raw data to match the circuits that the user implemented. |
jaygambetta
approved these changes
Nov 28, 2018
ajavadia
approved these changes
Nov 28, 2018
|
This looks great. |
Yes! I'll update the relevant issues (#830) |
lia-approves
pushed a commit
to edasgupta/qiskit-terra
that referenced
this pull request
Jul 30, 2019
* Replace qiskit.Result, remove tools/file_io Replace the existing `qiskit.Result` with the model-based class, previously in `qiskit.validation.result`, splitting it in two files for convenience. Not all functionality has been moved to the new file yet. Remove the utilities in file_io, as they are superseeded by the direct conversion of results objects from and to json. * Complete replacing of Result with model object Complete the replacing of `qiskit.Result` with the model-based class: * move the remaining old methods of `_result` to the new class, with extra comments for deciding on the ones that make the public API. * remove `qobj/_result.py` and `result/_result.py`. * revise `result_from_old_style_dict` * Update backends for new-style results Update the backends for the new-style results: * revise usage of `result_from_old_style_dict`, dropping the second parameter. * adjust statevector handling of snapshots. * adjust unitary creation of the vector, converting complex numbers to pairs. * Update tests for new style results Update a number of tests for the new style results: * use a temporary function for converting between count keys as binary to hex keys. * skipping a number of tests (tomography, result aggregation functions) marking them as follow-ups. * misc adjustments in a lot of places. * Revise bin_to_hex_keys usage Include `bin_to_hex` as part of `assertDictAlmostEqual`, in order to reduce the number of adjustments needed for the tests. * Update qiskit.ibmq for new style results Update the `qiskit.backends.ibmq` module in order to deal with new style results, adjusting tests accordingly. * Use results[x].header.name as circuit name Update the backends and the helper functions to ensure that the circuit name is available as `result.results[x].header.name`. Update examples accordingly. * Result API: deprecate get_status, circuit_statuses * Result API: deprecate result addition * Result API: deprecate get_ran_qasm, others Also get_circuit_status, get_job_id. * Result API: deprecate get_names, docstrings Deprecated `get_names()` and revise the docstrings of the non deprecated functions. * Result API: remove len() and indexing, linting Remove the `__len__` and `__getitem__` methods of Results, in turn removing checking for length and indexing. Those methods were not used in the tests (except in a couple of occurences), and deprecating was not as clean as with regular methods - this can be revised if needed. Lint and style tweaks. * Update CHANGELOG Removing again the "removed" duplicated section, adding entries. * Update CHANGELOG adding PR number * Update test_teleport with count keys
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:
specstag) - this updates the models so the dimensions ofunitaryandstatevectorare updated, matching Updated schemas to fix n_qubits and results statevector and unitary l… #1327Summary
This rather big PR consists of two parts:
simplify
qiskit.Resultinstantiation, removing the need for the intermediateqobj.result(Simplify Result instantiation #1266).The goal is to be able to instantiate a result either from its components (in
qobj.result.models, to be used in the simulators) or from a schema-conformant dict viaResult.from_dict()directly. Theqiskit.Resultmakes use of the machinery at Add auto-validated objects machinery #1249 so instances are always schema-conformant during creation.In turn, it ripples on making the simulators and online backends
Resultschema-conformant: the main source of changes is dealing with thecountskeys, that need to be in hex (0x123), with some minor changes in other places (unitary nesting, enforcing that the circuit names are always inresult.results[x].header.name). A conservative approach has been used (namely: do not modify the simulators too much in this PR, and still make use ofresult_from_old_style_dict) in order to make the PR more manageable.revising the Result "public API": this set of commits (the ~second part of the PR) follows Review qiskit.Result API #830, deprecating a number of functions and removing two of them (len() and indexing).
Please see the potential followups, as this PR was mostly aimed at producing a minimal set of changes that can be merged relatively quickly and improve it in subsequent PRs, unblocking other PRs and issues.
Details and comments
Followups:
Results from the models instead of from a dict (Local simulators should generate results according to the result schema #755, [WIP] Support per-shot measurements + unified result postprocessing #1279)countskeys in a format other than hex, for convenience. Ali's cool work on [WIP] Support per-shot measurements + unified result postprocessing #1279 is likely to be reused.bin_to_hex_dictfunction was introduced, just for convenience).