-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add a tool for measuring expectation values of Pauli strings with readout error mitigation #7067
base: main
Are you sure you want to change the base?
Conversation
…of qubits as input
… readout error mitigation
…CalibrationResult. Besides, fixed some lints
Hey @eliottrosenberg and @NoureldinYosri if you could take a look at this. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice work Danni!! This looks really great!
A few initial comments:
Regarding line 55 in shuffle_circuits_with_readout_benchmarking.py
, it should be possible to specify 0 for num_random_bitstrings
in order to do the measurement without readout mitigation (or have a different way of running without readout mitigation).
I think you went with lists of tuples for circuits_to_pauli
because you can't put a cirq.Circuit
into a dictionary, but you can put a cirq.FrozenCircuit
into a dictionary, so maybe that would be more convenient (not sure). Also, the object that measure_pauli_strings
outputs is pretty complicated, with lots of nested tuples, so maybe we want to create some data class for the output?
…_random_bitstrings, and add a test to cover the situation. The design is the run_shuffled_with_readout_benchmarking method will return a empty SingleQubitReadoutCalibrationResult if no calibration is actually done. 2. Allow measure_pauli_strings to take num_random_bitstrings = 0. In this case, no mitigation is actually done, and the mitigated result and unmitigated result are the same. Also add a test to handle this situation. 3. Make the return type of measure_pauli_strings a data class.
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, Danni!! I tried testing it in this colab and got the error shown there.
Can you try again? The problem should be fixed in the latest commit ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, Danni! I tested it again, and there seems to be a bug in the way it is computing the expectation value. I tried it on a simulator without noise in a case where the expectation value should be 1 and got something close to 0. https://colab.sandbox.google.com/drive/1_on4xIHQNMH_2km3RjlfoZxBcYim9A3y
Let me know if you have any trouble identifying the cause of this and I can step in and help.
@ddddddanni Here is a simpler example that illustrates the bug: https://colab.sandbox.google.com/drive/1DUOEbrJSHIvlIN1CI7T6lug4zGDB5nyG |
Thanks for catching this! This is due to the code doesn't cover the situation that some pauli strings could be pauli I. I will fix it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ddddddanni LGTM ... I will take another look after you fix the failing CIs
from cirq.study import ResultDict | ||
|
||
|
||
@dataclasses.dataclass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer to use attrs
over dataclasses
elif pauli_op == ops.Y: | ||
# Rotate to Y basis: Rx(pi/2) | ||
operations.append(ops.rx(np.pi / 2)(qid_list[qubit_index])) | ||
# No operation needed for Pauli Z or I (identity) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there shouldn't be identity in this case
# Extract unique qubit tuples from all circuits | ||
unique_qubit_tuples = set() | ||
for circuit in circuits_to_pauli.keys(): | ||
unique_qubit_tuples.add(tuple(sorted(set(circuit.all_qubits())))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
circuit.all_qubits()
returns a frozenset, so you can dop the set
here and elsewhere
Done! Can you try test again? The issue was Pauli I was incorrectly treated as Z in expectation calculation, and now the issue is fixed, and I also modified tests to test this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @ddddddanni! This is really great! One more request: can we multiply the result of measuring the PauliString by its coefficient? For example, right now your code does not distinguish between the following PauliStrings:
cirq.PauliString(cirq.Z.on_each(cirq.LineQubit.range(2)))
and
-1 * cirq.PauliString(cirq.Z.on_each(cirq.LineQubit.range(2)))
(And when you implement my previous suggestion, make sure to also scale the statistical uncertainty by |
This PR does the following two things:
Improved Readout Benchmarking: Modified run_shuffled_with_readout_benchmarking to accept a list of qubit tuples (List[Tuple[Qubit]]). This allows for calculating the readout error rate on specific qubit subsets, ensuring alignment with the qubit sets used in non-identity Pauli operators within Pauli strings.
Added a tool to compute expectation values for Pauli operators. This tool calculates both unmitigated and readout-error-mitigated expectation values for each circuit, and integrates the results from the readout benchmarking.