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

MPS::apply_multi_qubit_gate can crash in some circumstances #2292

Open
aromanro opened this issue Jan 16, 2025 · 2 comments
Open

MPS::apply_multi_qubit_gate can crash in some circumstances #2292

aromanro opened this issue Jan 16, 2025 · 2 comments
Labels
bug Something isn't working

Comments

@aromanro
Copy link
Contributor

aromanro commented Jan 16, 2025

Informations

  • Qiskit Aer version: Latest development version
  • Python version: Not relevant as qiskit aer was used from C++ when noticing it
  • Operating system: Windows 11

What is the current behavior?

I was testing the MPS simulator with some randomly generated circuits... with some I've got crashes.
This one I noticed after fixing #2291

The call chain was: AerState::apply_measure -> AerState::flush_ops -> Base::apply_ops -> MatrixProductState::State::apply_op -> MPS::apply_diagonal_matrix -> MPS::apply_matrix -> MPS::apply_multi_qubit_gate.

It crashed on this line:

new_mat(new_vec[row], new_vec[col]) = mat(row, col);

is_diagonal was true (as it should be by looking at the call chain), mat had 1 row and 8 columns (being a diagonal matrix, only the diagonal is given, so it's actually a vector)... but at that line it's used with mat(row, col) which is generating the crash.

Steps to reproduce the problem

Again, this might be quite tough to reproduce, I was generating some random circuits with more than 100 ops to test some things and maybe one in 100 circuits crashed.

What is the expected behavior?

No crash.

Suggested solutions

The problem lines should probably be replaced by something like:

      if (row == col)
        new_mat(new_vec[row], new_vec[row]) = is_diagonal ? mat(0, row) : mat(row, row);
      else
        new_mat(new_vec[row], new_vec[col]) = is_diagonal ? 0. : mat(row, col);

... or maybe a vector new_mat should be constructed if is_diagonal is true

@aromanro aromanro added the bug Something isn't working label Jan 16, 2025
aromanro added a commit to InvictusWingsSRL/qiskit-aer that referenced this issue Jan 16, 2025
aromanro added a commit to InvictusWingsSRL/qiskit-aer that referenced this issue Jan 16, 2025
aromanro added a commit to InvictusWingsSRL/qiskit-aer that referenced this issue Jan 16, 2025
@aromanro
Copy link
Contributor Author

It seems that the code that follows expects the matrix to be in a single row format if is_diagonal is true, so probably a better fix would be:

  cmatrix_t new_mat(is_diagonal ? 1 : sidelen, sidelen);

  if (is_diagonal) {
    for (uint_t col = 0; col < sidelen; ++col) {
      new_mat(0, new_vec[col]) =
          mat.GetRows() == 1 ? mat(0, col) : mat(col, col); // this is just in case something passes a matrix
                                                            // instead of a vector even if is_diagonal is true
    }
  } else {
    for (uint_t col = 0; col < sidelen; ++col) {
      for (uint_t row = 0; row < sidelen; ++row) {
        if (row == col)
          new_mat(new_vec[row], new_vec[row]) = mat(row, row);
        else
          new_mat(new_vec[row], new_vec[col]) = mat(row, col);
      }
    }
  }

@aromanro
Copy link
Contributor Author

if (row == col) is not necessary, so that should be removed as well.

aromanro added a commit to InvictusWingsSRL/qiskit-aer that referenced this issue Jan 16, 2025
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

1 participant