Add Grover and QAE result classes #1219
Conversation
manoelmarques
added
the
Changelog: API Change
manoelmarques
force-pushed
the
alg_result
branch
7 times, most recently
from
b667c0d
to
4085c72
Compare
manoelmarques
force-pushed
the
alg_result
branch
from
4085c72
to
9d58b94
Compare
|
@manoelmarques |
| self.data['ml_value'] = value | ||
|
|
||
| @property | ||
| def ret_values(self) -> List[float]: |
There was a problem hiding this comment.
I don't ret_values is the right name for this, these values are the points with a non-zero sampling probability. If values is too unspecific we could name it sampled_points or sampled_gridpoints. They go along with what's currently called probabilities.
Maybe it would be cleanest to just store the dictionary of {sampled_point: probability}. This is currently available as a_items but could be given a more sensible name (like sampled_points). y_items should be updated accordingly.
There was a problem hiding this comment.
@Cryoris can you and Stefan decide then on specific names and contents such that the new result objects are as wanted.
| def statevector(self) -> Union[None, np.ndarray]: | ||
| """ return statevector """ | ||
| return self.get('statevector') | ||
|
|
||
| @statevector.setter | ||
| def statevector(self, value: np.ndarray) -> None: | ||
| """ set statevector """ | ||
| self.data['statevector'] = value | ||
|
|
||
| @property | ||
| def counts(self) -> Union[None, Counts]: | ||
| """ return counts """ | ||
| return self.get('counts') | ||
|
|
||
| @counts.setter | ||
| def counts(self, value: Counts) -> None: | ||
| """ set counts """ | ||
| self.data['counts'] = value |
There was a problem hiding this comment.
What do you think of merging this into one output describing the result of the executed circuit, which is either the statevector or counts dict depending on the backend? It would clean up the object a little and it would be easier for display because we don't have to do a distinction on which backend was used on the user side.
There was a problem hiding this comment.
I merged both into cct_result property
| def mapped_values(self) -> List[float]: | ||
| """ return mapped_values """ | ||
| return self.get('mapped_values') | ||
|
|
||
| @mapped_values.setter | ||
| def mapped_values(self, value: List[float]) -> None: | ||
| """ set mapped_values """ | ||
| self.data['mapped_values'] = value |
There was a problem hiding this comment.
Should be updated accordingly with whatever the decision of ret_values/values is
|
@a-matsuo The property assignment is the variable 'assignment' in Grover that is currently the 'result' key in the returning dictionary. I thought that preserving the name 'assignment' in GroverResult was better than just calling it 'result' as it is currently in the dictionary. |
Co-authored-by: Julien Gacon <gaconju@gmail.com>
There was a problem hiding this comment.
Hopefully some of, what seem to me to be rather nondescript names, like 'a' and 'y' etc., can be refactored as part of the overall algorithm rework into something better. Maybe these single letters align with some paper but looking at them standalone they don't convey much on their own. This sort of change though extends to more than just the result object, that this PR was focused on.
|
Yeah agreed, we need to find more descriptive names for these. |
* Add Grover and QAE result classes * Update qiskit/aqua/algorithms/amplitude_estimators/mlae.py Co-authored-by: Julien Gacon <gaconju@gmail.com> * Change return from Union[None, ...] to Optional[...] * Combine statevector and counts into cct_result * change names Co-authored-by: Julien Gacon <gaconju@gmail.com>
* Add Grover and QAE result classes * Update qiskit/aqua/algorithms/amplitude_estimators/mlae.py Co-authored-by: Julien Gacon <gaconju@gmail.com> * Change return from Union[None, ...] to Optional[...] * Combine statevector and counts into cct_result * change names Co-authored-by: Julien Gacon <gaconju@gmail.com>
* Add Grover and QAE result classes * Update qiskit/aqua/algorithms/amplitude_estimators/mlae.py Co-authored-by: Julien Gacon <gaconju@gmail.com> * Change return from Union[None, ...] to Optional[...] * Combine statevector and counts into cct_result * change names Co-authored-by: Julien Gacon <gaconju@gmail.com>
* Add Grover and QAE result classes * Update qiskit/aqua/algorithms/amplitude_estimators/mlae.py Co-authored-by: Julien Gacon <gaconju@gmail.com> * Change return from Union[None, ...] to Optional[...] * Combine statevector and counts into cct_result * change names Co-authored-by: Julien Gacon <gaconju@gmail.com>
Summary
Related to #1068
Details and comments