(2/2) Combine Pauli measurements and postselection#7970
(2/2) Combine Pauli measurements and postselection#7970ddddddanni wants to merge 10 commits intoquantumlib:mainfrom
Conversation
|
I tested using https://colab.sandbox.google.com/drive/1xY5N6NBm2kSL_kChUr566hLNlwiNApY6?resourcekey=0-D_sl5TxF7eo9A_azwwb8nQ#scrollTo=w8fWdb-2Ki6c, results seem to be fine |
eliottrosenberg
left a comment
There was a problem hiding this comment.
LGTM! Thanks, @ddddddanni!
Some testing in this colab.
eliottrosenberg
left a comment
There was a problem hiding this comment.
Actually, maybe the docstrings could make it a bit clearer that it doesn't perform readout benchmarking if postselection symmetries are included and that the parameters readout_repetitions and num_random_bitstrings are ignored in this case.
Done! I added this: https://screenshot.googleplex.com/6B7SV7HRxZWyH9C |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7970 +/- ##
========================================
Coverage 99.63% 99.63%
========================================
Files 1110 1110
Lines 99738 99855 +117
========================================
+ Hits 99376 99493 +117
Misses 362 362 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Please check if we can avoid some code repetition (see inline).
We should also check if measure_pauli_strings can have a simpler interface.
@eliottrosenberg - is it required for measure_pauli_strings to handle an argument with a mixture of post-selection-symmetry and confusion-matrix cases?
| Note: If `postselection_symmetries` are included in the `circuits_to_pauli` parameters, | ||
| the circuit will be measured using the post-selection symmetry method. | ||
| In this case, the `readout_repetitions` and `num_random_bitstrings` arguments are ignored. |
There was a problem hiding this comment.
This is a bit over-complicated interface. Both readout_repetitions and num_random_bitstrings arguments are mandatory, but unused for the post-selection symmetry case.
Would it work to include readout_repetitions and num_random_bitstrings in CircuitToPauliStringsParameters where they would be mutually exclusive with postselection_symmetries and the split this interface to 2 overloads:
1 legacy interface (if feasible deprecate):
@overload
def measure_pauli_strings(
circuits_to_pauli: (
Mapping[circuits.FrozenCircuit, Sequence[ops.PauliString]
| Sequence[Sequence[ops.PauliString]]]
),
sampler: work.Sampler,
pauli_repetitions: int,
readout_repetitions: int,
num_random_bitstrings: int,
...
)2 preferred interface (readout_repetitions, num_random_bitstrings in CircuitToPauliStringsParameters)
@overload
def measure_pauli_strings(
circuits_to_pauli: list[CircuitToPauliStringsParameters],
sampler: work.Sampler,
pauli_repetitions: int,
...
)There was a problem hiding this comment.
I'm not sure if it's better to put readout_repetitions and num_random_bitstrings under CircuitToPauliStringsParameters, because these two parameters are more like global parameters that could take effects to all circuits, while CircuitToPauliStringsParameters is only for one circuit. wdyt?
This commit implements the postselection symmetry functionality in
CircuitToPauliStringsParameters. Circuits provided withpostselection_symmetrieswill now successfully filter measurement results based on the expected eigenvalues rather than defaulting to the confusion matrix method.