-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Allow users to run readout benchmarking with sweep #7435
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7435 +/- ##
========================================
Coverage 98.69% 98.69%
========================================
Files 1112 1112
Lines 98076 98199 +123
========================================
+ Hits 96798 96921 +123
Misses 1278 1278 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cc: @eliottrosenberg this is a smaller RP I split from #7358. This one only touches the readout benchmarking but not pauli expectation calculation. |
@@ -84,6 +101,79 @@ def _generate_readout_calibration_circuits( | |||
return readout_calibration_circuits, random_bitstrings | |||
|
|||
|
|||
def _generate_parameterized_readout_calibration_circuit_with_sweep( | |||
qubits: list[ops.Qid], rng: np.random.Generator, num_random_bitstrings: int |
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.
traditionally rng
should be the last input, same below
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.
Done!
qubits_set: set[ops.Qid] = set() | ||
for circuit in input_circuits: | ||
qubits_set.update(circuit.all_qubits()) | ||
qubits_to_measure = [sorted(qubits_set)] |
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.
nit: this can be a single line
qubits_to_measure = [sorted(qubits_set)] | |
qubits_to_measure = [sorted(set(q for circuit in input_circuits for q in 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.
Done!
|
||
|
||
def run_sweep_with_readout_benchmarking( | ||
input_circuits: list[circuits.Circuit], |
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.
the sampler should be the first parameters + there are a lot of parameters here, consider grouping them into a data class this will allow you to move the input validation logic to that data class, for example
@attrs.frozen
class ReadoutBenchmarkingSweepParams:
circuit_repetitions: Union[int, list[int]]
num_random_bitstrings: int = 100
readout_repetitions: int = 1000
qubits: Optional[Union[Sequence[ops.Qid], Sequence[Sequence[ops.Qid]]]] = None
def __attrs_post_init__(self):
# validate inputs
def run_sweep_with_readout_benchmarking(
sampler: work.Sampler,
input_circuits: list[circuits.Circuit],
sweep_params: Sequence[study.Sweepable],
parameters: ReadoutBenchmarkingSweepParams,
rng_or_seed: Union[np.random.Generator, int],
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.
Sure! I'm not sure if you also want me to change the parameters in the run_shuffled_with_readout_benchmarking method? I'm worried this one (run_shuffled_with_readout_benchmarking) is already in used by someone.
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.
since it already exists, you can create a new method to do that functionality and add a deprecation method @deprecated
to the old method that points to the new method
Cirq/cirq-core/cirq/_compat.py
Line 290 in 609d93d
def deprecated( |
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.
Done! I deprecated the existing one and created a new method to do the functionality.
def _generate_parameterized_readout_calibration_circuit_with_sweep( | ||
qubits: list[ops.Qid], rng: np.random.Generator, num_random_bitstrings: int | ||
) -> tuple[circuits.Circuit, study.Sweepable, np.ndarray]: | ||
"""Generates a parameterized readout calibration circuit, sweep parameters, |
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.
either add more details to these docstrings or add more comments to the code to provide enough information for people who will read this code a year from now
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.
Done! Added some docstrings to make the method more clear.
Requested by @NoureldinYosri, this pr is splited from #7358.
The pr adds a new function run_sweep_with_readout_benchmarking which runs the sweep circuits with readout error benchmarking (without shuffling).