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

Unnecessary zero-probability states returned by Sampler, with different behaviour to Aer Sampler #9178

Closed
luciacuervovalor opened this issue Nov 22, 2022 · 6 comments · Fixed by #9248
Assignees
Labels
bug Something isn't working good first issue Good for newcomers mod: primitives Related to the Primitives module

Comments

@luciacuervovalor
Copy link
Contributor

Environment

  • Qiskit Terra version: 0.23.0.dev0+ad07847
  • Python version: 3.10.4
  • Operating system: macOS Monterey 12.6 M1 2020

What is happening?

  • Sampler returns a dictionary with every possible state, even if some states have a probability 0 of being sampled. As the number of qubits is increased, the size of this dictionary will scale exponentially.
  • The behaviour is different to the qiskit_aer Sampler, which does not return states with probability 0. Importantly, since the number of qubits is not saved, this results in a faulty conversion to binary strings in qiskit_aer using the binary_probabilities() method. This is probably because the length of the binary strings is calculated based on the number of returned states instead of the number of qubits, which in Aer is a subset of all possible states.

How can we reproduce the issue?

from qiskit_aer.primitives import Sampler as aer_sampler
from qiskit.primitives import Sampler as terra_sampler

qc = QuantumCircuit(4)
qc.h(0)
qc.measure_all()

terra_job = terra_sampler().run([qc], [], shots=10)
terra_quasis = terra_job.result().quasi_dists[0]
terra_bin_probs = terra_quasis.binary_probabilities()
print('TERRA RESULTS:')
print('\n', terra_quasis)
print('\n', terra_bin_probs)

aer_job = aer_sampler(backend_options={"method": "statevector"}).run([qc], [], shots=100)
aer_quasis = aer_job.result().quasi_dists[0]
aer_bin_probs = aer_quasis.binary_probabilities()
print('\nAER RESULTS:')
print('\n', aer_quasis)
print('\n', aer_bin_probs)

TERRA RESULTS:

{0: 0.5, 1: 0.5, 2: 0.0, 3: 0.0, 4: 0.0, 5: 0.0, 6: 0.0, 7: 0.0, 8: 0.0, 9: 0.0, 10: 0.0, 11: 0.0, 12: 0.0, 13: 0.0, 14: 0.0, 15: 0.0}

{'0000': 0.5, '0001': 0.5, '0010': 0.0, '0011': 0.0, '0100': 0.0, '0101': 0.0, '0110': 0.0, '0111': 0.0, '1000': 0.0, '1001': 0.0, '1010': 0.0, '1011': 0.0, '1100': 0.0, '1101': 0.0, '1110': 0.0, '1111': 0.0}

AER RESULTS:

{1: 0.54, 0: 0.46}

{'1': 0.54, '0': 0.46}

What should happen?

We should see only the non-zero probability states in Terra, and a correct conversion to binary in Aer (i.e. matches the number of qubits):

TERRA RESULTS:

{0: 0.5, 1: 0.5}

{'0000': 0.5, '0001': 0.5}

AER RESULTS:

{1: 0.54, 0: 0.46}

{'0001': 0.54, '0000': 0.46}

Any suggestions?

  • Since the "statevector" method in Aer doesn't return zero probability states, the same steps could be applied to the Statevector in Terra (which is called by the Sampler).
  • The number of qubits could be saved in the metadata of the SamplerResult, such that Aer can properly do the conversion to binary probabilities.
@luciacuervovalor luciacuervovalor added the bug Something isn't working label Nov 22, 2022
@woodsp-ibm woodsp-ibm added the mod: primitives Related to the Primitives module label Nov 22, 2022
@t-imamichi
Copy link
Member

Thank you for your report.

Aer passes bitstrings as integer to QuasiDistribution. So, the result looses the information of number of qubits.
I made a simple PoC to fix the issue for shots is not None Qiskit/qiskit-aer#1668.
@ikkoham can elaborate this patch.

It might be good to remove results with zero probability for the reference implementation of Sampler though we don't guarantee the behavior. It may be a good first issue.

@Cryoris
Copy link
Contributor

Cryoris commented Nov 30, 2022

Couldn't the QuasiDistribution just contain the number of bits? It already contains the number of shots so it's already strictly tied to a quantum circuit result 😄 because this exponential scaling would be nice to avoid if e.g. someone is iterating over the outcomes.

@t-imamichi
Copy link
Member

Yes, it might be good to remove bits with 0 prob at QuasiDistribution side. It can be a good first issue too.

@ikkoham ikkoham added the good first issue Good for newcomers label Dec 1, 2022
@ikkoham
Copy link
Contributor

ikkoham commented Dec 1, 2022

Also Terra's Sampler should use probabilities_dict method instead of probabilities.
https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/quantum_info/states/quantum_state.py#L216

@javabster javabster moved this to Tagged but unassigned in Contributor Monitoring Dec 2, 2022
@nonhermitian
Copy link
Contributor

This would be nice to fix.

@t-imamichi
Copy link
Member

@mergify mergify bot closed this as completed in #9248 Jan 19, 2023
@github-project-automation github-project-automation bot moved this from Tagged but unassigned to Done in Contributor Monitoring Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers mod: primitives Related to the Primitives module
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants