-
Notifications
You must be signed in to change notification settings - Fork 134
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
MinimumEigenOptimizer returns a wrong solution #92
Comments
Do we need to reverse the solution bitstring or would it maybe be better to reverse the bits in the Hamiltonian? Then the solution with the If we only flip the solution in |
Yes, it will be fixed in #97 |
You're right. I only flipped the solution in |
Oh, sorry. I missed the point. We need to revisit the ordering of Hamiltonian. How about discussing it in another issue? |
Why not in this issue (since the bug is described here)? 🙂 If the changes are not too big it would probably be better to do only one fix instead of first re-ordering the solution bits and then undoing this by re-ordering the Hamiltonian. @a-matsuo do you think you could check if reversing the Hamiltonian order fixes the problem consistently? 🙂 |
It's just in case. Some users may use the current order. |
But, as you say, it's a good opportunity to change the order because this library is new. |
Anyways, we need to clarify the order of Hamiltonian in docstrings of |
@Cryoris Sure. Let me check reversing the Hamiltonian order works correctly as I expect. I will work on it next week. |
If we reverse the Hamiltonian ordering, |
I'm checking some PRs related to endian and wondering what is the best option. |
I think all of the qiskit operators (including PauliSumOp) and circuits should be little endian if possible. Second quantized operators in qiskit-nature is only exception since the application users are familiar with BIG endian. |
From the perspective of operators, when users manually make Ising Hamiltonians for optimization problems, it is natural to think mapping qubit 1 to variable 1. I think the current way (mapping x1 to q1) makes sense.
Then, for the above issue, maybe, writing clear docstrings is the only solution? |
Agree. It's good to add a docstring about mapping of variables to qubits. |
I don't think it's a good long term solution to have different qubit ordering in Qiskit Core and in the application modules. This will require users to exactly know in which part of Qiskit they are operating and swap qubit orders in the right places -- which can even be confusing for us developers! 😄 It's true that many users are used to big endian notation, but Qiskit just doesn't use big endian so I would argue it's better to be clear about this and not have different conventions in on software package. As a compromise, we could add a |
Yes. So, let's keep the current qubit order (variable_i -> qubit_i) to align with qiskit core's convention and clarify it in the docstring of The endian option sounds interesting. I would like to discuss it as a future work. |
I updated the docstrings of |
Ok 👍🏻 yeah let's revisit the ordering in another issue, I'm not sure I fully understood where which order is assumed 😉 Thanks for the clarifications! |
Information
What is the current behavior?
MinimumEigenOptimizer
with qasm_simulator returns a wrong solution.Steps to reproduce the problem
output:
What is the expected behavior?
The solution of VQE should be
[0. 1.]
.Suggested solutions
We may need to reverse the bitstrings as
x = np.fromiter(list(bitstr[::-1]), dtype=int)
. But, it may affect other algorithms and unit tests.https://github.com/Qiskit/qiskit-optimization/blob/54a6d0750492132aa019975e3a8648c507b7553c/qiskit_optimization/algorithms/optimization_algorithm.py#L511
The text was updated successfully, but these errors were encountered: