Qiskit / qiskit-aer

Aer is a high performance simulator for quantum circuits that includes noise models
https://qiskit.github.io/qiskit-aer/
Apache License 2.0
505 stars 364 forks source link

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

Open aromanro opened 2 months ago

aromanro commented 2 months ago

Informations

Latest dev from main branch.

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

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 https://github.com/Qiskit/qiskit-aer/issues/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.

hhorii commented 2 months ago

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.

aromanro commented 2 months ago

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

aromanro commented 2 months ago

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 commented 2 months ago

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