Fix shots argument of local_readout_mitigator

[peendebak]

Summary

• The LocalReadoutMitigator.quasi_probabilities method has an argument shots which is not used in the method. The docstring us updated for this
• The method returns an object of type QuasiDistribution. The number of shots is set on that object
• The documentation on the return type of counts_probability_vector is updated.

Fixes #9154

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

[coveralls]

Pull Request Test Coverage Report for Build 3544051628

• 6 of 6 (100.0%) changed or added relevant lines in 2 files are covered.
• 7 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+0.05%) to 84.556%

---

Files with Coverage Reduction	New Missed Lines	%
qiskit/extensions/quantum_initializer/squ.py	2	79.78%
src/sabre_swap/layer.rs	2	98.95%
src/vf2_layout.rs	3	94.74%

Totals
Change from base Build 3542917032:	0.05%
Covered Lines:	62843
Relevant Lines:	74321

---

💛 - Coveralls

[Cryoris]

I'm not an expert here, but I'm not sure this makes sense: in QuasiDistribution the number of shots is provided as integer. Here it is a boolean? It seems like either the argument type here should also be an integer, otherwise it cannot be safely passed to the QuasiDistribution.

[peendebak]

I'm not an expert here, but I'm not sure this makes sense: in QuasiDistribution the number of shots is provided as integer. Here it is a boolean? It seems like either the argument type here should also be an integer, otherwise it cannot be safely passed to the QuasiDistribution.

The argument shots of the quasi_probabilities method is annotated as a boolean, but it is was not used at all. In the base class it is indeed annotated as an int. I modified the PR to set the number of shots as an int.

If the value of shots is not None, then it is used as is, otherwise it is calculated as:

probs_vec, shots = counts_probability_vector(
            data, qubit_index=self._qubit_index, clbits=clbits, qubits=qubits
        )

I am not sure what the value of a non-default value for the shots argument is. @ShellyGarion You introduced this code in #6485. Why do we need the shots argument in the quasi_probabilities method?

[ShellyGarion]

The shots are needed to calculate the stddev of the mitigated expectation value.
https://github.com/Qiskit/qiskit-terra/blob/4a1a2d0a79ba8cac77275326a32da117e9663c05/qiskit/result/mitigation/local_readout_mitigator.py#L277
https://github.com/Qiskit/qiskit-terra/blob/4a1a2d0a79ba8cac77275326a32da117e9663c05/qiskit/result/mitigation/local_readout_mitigator.py#L163

It's not necessary, hence it's type should be optional.

[ShellyGarion]

I think that one should fix this: https://github.com/Qiskit/qiskit-terra/blob/4a1a2d0a79ba8cac77275326a32da117e9663c05/qiskit/result/mitigation/local_readout_mitigator.py#L172

to shots: Optional[int] = None

as in the base mitigator class:
https://github.com/Qiskit/qiskit-terra/blob/4a1a2d0a79ba8cac77275326a32da117e9663c05/qiskit/result/mitigation/base_readout_mitigator.py#L32

[gadial]

I'm not sure I understand what user-supplied shots actually mean in this context. The number of shots can be computed (as you do) from the counts data; passing other shots value not consistent with the count data cannot yield meaningful results, unless I'm missing something.

Currently I think the parameter should simply be omitted.

[peendebak]

@gadial @ShellyGarion I will remove the shots argument, as it can be calculated from the data and any other value would not make sense.

Should we remove it altogether, or add a deprecation warning?

[ShellyGarion]

If you remove shots then it will require a deprecation warning, since it's part of the API:
https://qiskit.org/documentation/stubs/qiskit.result.LocalReadoutMitigator.quasi_probabilities.html#qiskit.result.LocalReadoutMitigator.quasi_probabilities

However, I am not sure if the number of shots can always be calculated from the counts, as the Counts object does not necessarily have to be a dictionary of integers (it can also contain probabilities or quasi-probabilities, I think).

[peendebak]

If you remove shots then it will require a deprecation warning, since it's part of the API: https://qiskit.org/documentation/stubs/qiskit.result.LocalReadoutMitigator.quasi_probabilities.html#qiskit.result.LocalReadoutMitigator.quasi_probabilities

However, I am not sure if the number of shots can always be calculated from the counts, as the Counts object does not necessarily have to be a dictionary of integers (it can also contain probabilities or quasi-probabilities, I think).

Ok, in that case I will leave keep the argument (just update the typing from bool to int)

[ShellyGarion]

@peendebak - thanks for fixing this!
Perhaps it's worth to add a test to check that shots is handled OK now?
The tests are located here: https://github.com/Qiskit/qiskit-terra/blob/main/test/python/result/test_mitigators.py

[ShellyGarion]

LGTM. Thanks for fixing this!

[jakelishman]

Please could we add a bugfix release note as well?

[jakelishman]

Perfect, thanks!