Skip to content
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

AerState::probability and AerState::amplitude calls for a MPS simulator return the wrong ordering for states #2235

Open
aromanro opened this issue Sep 18, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@aromanro
Copy link
Contributor

aromanro commented Sep 18, 2024

Informations

  • Qiskit Aer version:

Latest dev from main branch.

  • Python version:

Irrelevant as it's called from c++ code.

  • Operating system:

Windows 11

What is the current behavior?

I have a c++ project where I have a test that compares results of my implementation of a MPS simulator with the qiskit aer MPS one. I simply generated random circuits which I applied on the simulators, expecting them to have similar results afterwards.

I tried various approaches for comparisons, one that works on windows (but for some reason crashes on linux) is by using move_to_vector() to be able to compare amplitudes... so I tried to switch to some other ways of getting amplitudes/probabilities, to avoid the crash on linux.

As a workaround, I tried to go over all base states and call amplitude and/or probability. It turns out that the results do not come as expected, the values seem to be correct but not in the proper order. I know that the MPS simulator must bring together the qubits in order to apply the two or more qubit gates, so it has to map the qubits order as expected from 'outside' to some internal ordering... and I think in there something is not ok when doing the conversion for the amplitude/probability calls.

Steps to reproduce the problem

Execute a circuit with a statevector simulator and the same one with a MPS simulator, try to compare results with amplitude/probability calls.

Another way would be (after fixing #2234) to try to compare the vector obtained by a call to probabilties() against values obtained for each basis state with probability or with the norm of the value obtained by amplitude.

What is the expected behavior?

Have the same ordering as for the statevector, for example.

Suggested solutions

I couldn't figure out the cause yet, but move_to_vec seems to return the expected statevector, maybe this could help.
On the other hand, for some reason move_to_vec crashes for me on linux... I didn't look into that one yet, I thought I could avoid it by using the mentioned calls.

@aromanro aromanro added the bug Something isn't working label Sep 18, 2024
@hhorii
Copy link
Collaborator

hhorii commented Sep 18, 2024

AerState is an internal class for statevector (and density matrix) and we do not tested it for MPS. I will do some quick investigation why probabilties() does not work as expected but cannot guarantee we fix this issue.

@hhorii hhorii self-assigned this Sep 18, 2024
@aromanro
Copy link
Contributor Author

Wait a little bit, I think I fixed it... I'll commit something and then add a comment on what I did.

aromanro added a commit to InvictusWingsSRL/qiskit-aer that referenced this issue Sep 18, 2024
@aromanro
Copy link
Contributor Author

A quick and dirty fix. I just modified MPS::get_amplitude_vector by calling another function for reordering:

uint_t actual_base_value =
        reorder_qubits_rev(qubit_ordering_.order_, base_values[i]);

Then I added that function simply by copying and altering reorder_qubits, like this:

uint_t reorder_qubits_rev(const reg_t &qubits, uint_t index) {
  uint_t new_index = 0;

  int_t current_pos = 0, current_val = 0, new_pos = 0, shift = 0;
  uint_t num_qubits = qubits.size();
  for (uint_t i = 0; i < num_qubits; i++) {
    current_pos = qubits[i];
    current_val = 1ULL << current_pos;
    new_pos = i;
    shift = new_pos - current_pos;
    if (index & current_val) {
      if (shift > 0) {
        new_index += current_val << shift;
      } else if (shift < 0) {
        new_index += current_val >> -shift;
      } else {
        new_index += current_val;
      }
    }
  }
  return new_index;
}

The changes are that I simply removed num_qubits - 1 - from the assignments for current_pos and new_pos.

@aromanro
Copy link
Contributor Author

aromanro commented Sep 18, 2024

Simply altering the original function isn't going to work since that is used elsewhere, too, and it will break some other things.

aromanro added a commit to InvictusWingsSRL/qiskit-aer3 that referenced this issue Sep 21, 2024
…lls for a MPS simulator return the wrong ordering for states"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants