Remove zero-probability states from Sampler's result

[t-imamichi]

Summary

Fixes #9178

Requires #9258 to pass the unit tests

Details and comments

[qiskit-bot]

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

• @Qiskit/terra-core
• @ajavadia
• @ikkoham
• @levbishop
• @t-imamichi

[Cryoris]

LGTM, could you add a reno since this changes the results? 🙂

[t-imamichi]

Sure. I also need to fix some tests and a sampler gradient.

[t-imamichi]

I added a reno

[coveralls]

Pull Request Test Coverage Report for Build 3956431316

• 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
• 8 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.002%) to 84.926%

---

Files with Coverage Reduction	New Missed Lines	%
src/vf2_layout.rs	8	86.44%

Totals
Change from base Build 3954112093:	0.002%
Covered Lines:	66541
Relevant Lines:	78352

---

💛 - Coveralls

[Cryoris]

Now that 0 probabilities are not included, shouldn't this just show the "11" key here? That also seems to be what the error is about 🙂

[t-imamichi]

Ideal case should include "00", "01", and "10" with very small probabilities. The test on Linux passes but that on Mac fails. I'm investigating the difference.

[Cryoris]

I cannot reproduce the CI failure locally on MacOS with Python 3.7 🤔

The values with which "00", "01" and "10" are included are about 1e-34 maybe we don't really need to check that this is recognized as non-zero? This seems a bit volatile to me... since this is about algorithm performance rather than the technicality which element is populated and from the sampler tests we know that the zero-probability states are removed, how about just checking if the values are correct in the Grover test? E.g. like

if i in (0, 3):
    values = [dist.get(key, 0) for key in ["00", "01", "10", "11"]]
    np.testing.assert_allclose(values, [0, 0, 0, 1], atol=0.2)

[jlapeyre]

I makes sense to me to filter out probabilities like this:

For the exact distribution: Remove probabilities satisfying $p_i \ll 1/|p| $. The reasoning is that the sum of any set of $p_i$ s of this magnitude is negligible. On the other hand we may want to just remove what we think is in some sense a numerical artifact. This might not be easy to determine. For example 1e-34 is probably safe to remove. But the former at least has a reason behind the choice. You still have to decide what is much less than one. Maybe 1e-10

For the distribution from samples, you should remove a probability if $p_i / n_\text{shots} \ll 1$. This will make sampling efficient.

But you want to do something to remove small probabilities in both cases. The dicts could otherwise consume a lot of memory and be populated mostly with garbage.

[ikkoham]

I will use decimals of Statevector.probabilities to round the results.

[jlapeyre]

EDIT: After looking more closely, I made further comments along the same lines, above.

This looks good. I have one comment. I think we need the ability to set a threshold, which is supported by the method probabilities_dict. For example, even when there is no noise, a QFT can result in amplitude that decays with distance from peaks.

I suppose that could be added to the API later.

EDIT: Rereading the discussion above, it looks like this is already an issue in the tests.

The code in quantum_info that removes zeros actually does rounding. It then checks the floating point results for equality with zero. Instead, it seems a little more to-the-point to generate bools, like this

inds = (np.abs(x) > tol).nonzero()[0]
return {i: x[i] for i in inds} 

This will leave all of the accepted values unchanged. Dunno, maybe there is a good reason to also round them. Using bools is a bit faster. But not so much to make a decision based on efficiency.

But probably the best place to make that change (if at all) is in quantum_info .

[ikkoham]

I'm trying to pass the CI right now, but it's quite a struggle.

[ikkoham]

LGTM