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

Code that never executes in MPS::sample_measure_single_qubit #2305

Open
aromanro opened this issue Feb 2, 2025 · 3 comments
Open

Code that never executes in MPS::sample_measure_single_qubit #2305

aromanro opened this issue Feb 2, 2025 · 3 comments
Labels
enhancement New feature or request

Comments

@aromanro
Copy link
Contributor

aromanro commented Feb 2, 2025

Informations

  • Qiskit Aer version: Latest development version
  • Python version: Doesn't matter, it's in the C++ source
  • Operating system: All

What is the current behavior?

There is some code in src/simulators/matrix_product_state/matrix_product_state_internal.cpp' in MPS::sample_measure_single_qubit` that never executes, more precisely, here:

if (qubit != 0) // multiply mat by left lambda

Steps to reproduce the problem

I was looking over the sources to learn more about the qiskit aer internals when I've spotted this. Might be harmless, but it looks suspicious, so I labelled it as a bug.

What is the expected behavior?

I need to look more carefully over the code and perhaps over the referred paper (and also over 'Andrew J. Ferris and Guifre Vidal, “Perfect sampling with unitary tensor networks,” Phys. Rev. B 85, 165146 (2012)') to be sure...

Suggested solutions

Either the

    if (qubit != 0) // multiply mat by left lambda
      for (uint_t col = 0; col < mat.GetColumns(); col++)
        for (uint_t row = 0; row < mat.GetRows(); row++)
          mat(row, col) *= lambda_reg_[qubit - 1][row];

code should not be there or it should not be there but the

      for (uint_t col = 0; col < mat.GetColumns(); col++)
        for (uint_t row = 0; row < mat.GetRows(); row++)
          mat(row, col) *= lambda_reg_[qubit - 1][row];

should be in the else branch.

@aromanro aromanro added the bug Something isn't working label Feb 2, 2025
aromanro added a commit to InvictusWingsSRL/qiskit-aer that referenced this issue Feb 5, 2025
aromanro added a commit to InvictusWingsSRL/qiskit-aer that referenced this issue Feb 6, 2025
aromanro referenced this issue in InvictusWingsSRL/qiskit-aer Feb 6, 2025
…Lambdas are multiplied into mat as the algorithm goes over the qubits in the 'multiply mat by right lambda' portion of the code.
@aromanro
Copy link
Contributor Author

aromanro commented Feb 6, 2025

Without yet looking over the referred article(s), it looks like the multiplication by left lambda should not be done, all lambdas are multiplied along into `mat' by the code that follows and multiplies the 'right lambda'.

This does not seem to be a bug, but simply a small enhancement by removing the useless code. Related with this, the shots parameter for the sample_measure function is useless and it should be removed.
Also I think that the documentation states that the complexity is O(1) for a shot, which is not true as the matrices are multiplied together for each shot.

Could somebody change the label to 'enhancement', it seems that I cannot edit that.

aromanro added a commit to InvictusWingsSRL/qiskit-aer8 that referenced this issue Feb 8, 2025
@gadial gadial added enhancement New feature or request and removed bug Something isn't working labels Feb 9, 2025
@gadial
Copy link
Collaborator

gadial commented Feb 9, 2025

This code was added in #1377, specifically in da1ff73. Originally the first check was whether we have is_first_qubit==True , where is_first_qubit was a parameter to the method, so the dead code might have been activated. In the current state I think it indeed can be removed.

@aromanro
Copy link
Contributor Author

aromanro commented Feb 9, 2025

Related with the same code, I think there still is an enhancement possible, avoiding the same matrix multiplication twice if zero is measured on a qubit.

What I mean is exemplified - because it's in a single function - in my implementation of a MPS simulator:
https://github.com/aromanro/QCSim/blob/798e33092d11db1053376e29207d3a3c914858a1/QCSim/MPSSimulatorImpl.h#L136
more precisely, this: https://github.com/aromanro/QCSim/blob/798e33092d11db1053376e29207d3a3c914858a1/QCSim/MPSSimulatorImpl.h#L168

gadial pushed a commit that referenced this issue Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants