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
Fix bit ordering and probabilities of samples in optimization_algorithm.py #97
Conversation
We don't need to wait for #96 because CI builds Aer on Ubuntu. |
Could you add bit ordering test of QAOA just in case? |
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!
Does this in anyway depend on Qiskit/qiskit#6365 ? Since all the unit tests pass here I am guessing not, but that is another reversal of order right? |
That's another reversal of order, but actually this does not depend on Qiskit/qiskit#6365. It's simply a bug caused by inconsistency. Currently, |
…hm.py (#97) * Fix bit ordering of `MinimumEigenOptimizer` with qasm_simulator. * Fix probabilities of solution samples with qasm_simulator. Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com> Co-authored-by: Takashi Imamichi <t.imamichi@gmail.com> Co-authored-by: Manoel Marques <manoel.marques@ibm.com> Co-authored-by: Takashi Imamichi <imamichi@jp.ibm.com> (cherry picked from commit 4bce259) # Conflicts: # qiskit_optimization/algorithms/grover_optimizer.py # qiskit_optimization/algorithms/optimization_algorithm.py # test/algorithms/test_grover_optimizer.py # test/algorithms/test_min_eigen_optimizer.py
Summary
Fixes #92
Details and comments
This PR should be merged after #96
Bit ordering
We need to reverse bit ordering from qiskit bit ordering to conventional bit ordering for the actual outputs.
Probabilities of samples
This PR fixes the following lines that calculate the probabilities of solutions.
When
eigenvector
is a dict, we need to square the values since the values are normalized.https://github.com/Qiskit/qiskit-optimization/blob/724158ba42f4b75758edc1de42f6f665660ec27e/qiskit_optimization/algorithms/optimization_algorithm.py#L554-L557
I will explain it in details for the record.
Originally, when we were using Qiskit Aqua, this
eigenvector
was supposed to be the resultantcounts
of a qasm simulator. Thus, the above code was working correctly.https://github.com/Qiskit/qiskit-aqua/blob/aa8bc0c8011405d382fc05d6ab576dd926bdd2a7/qiskit/aqua/algorithms/minimum_eigen_solvers/vqe.py#L578
However, after the code was moved into Qiskit terra, there was some bug and it was fixed in Qiskit/qiskit#5496 .
As a result, this
eigenvector
became the square root of the normalized counts, i.e. from{bitstr: counts}
to{bitstr: np.sqrt(counts / shots)}
.https://github.com/Qiskit/qiskit-terra/blob/3c979ebdacecbbd213757a8149bddc309bfa58c0/qiskit/algorithms/minimum_eigen_solvers/vqe.py#L520
If we change the value of this
eigenvector
in Qiskit terra in the future, we need to change the above code inMinimumEigenSolvers
as well.