Simultaneous readout #4307

dstrain115 · 2021-07-09T20:38:04Z

Adds an experiment for simultaneous (parallel) readout.

This experiment initializes a number of bitstrings(trials) and initializes the qubits to those bitstrings and immediately measures them It then estimates zero state and one state errors averaged over all of the trials where the given qubit was initialized to |0> or |1> respectively.

This change also deprecates SingleQubitReadoutResult in favor of an equivalent but more generally named ReadoutExperimentResult.

MichaelBroughton

Do you think this code could have been worked in a little more harmoniously with pre-existing simultaneous readout calibration routines which employed a slightly different methodology instead of creating two things that have very similar names that do only slightly different things ? The estimate_single_qubit_readout_errors works in a parallel setting, but it does not use random bitstrings like this method. Could we maybe add a flag to it that tells it to use random bitstrings or not use random bitstrings and then just need one function for this instead of two ?

dstrain115 · 2021-07-12T23:25:30Z

Do you think this code could have been worked in a little more harmoniously with pre-existing simultaneous readout calibration routines which employed a slightly different methodology instead of creating two things that have very similar names that do only slightly different things ? The estimate_single_qubit_readout_errors works in a parallel setting, but it does not use random bitstrings like this method. Could we maybe add a flag to it that tells it to use random bitstrings or not use random bitstrings and then just need one function for this instead of two ?

What do you think of this now? estimate_single_qubit_readout_errorsnow just calls the parallel version with specific parameters, thus preserving backwards compatibility and also simplifying the code.

(Note to self, I need to add a test/check for bit_strings that do not have a |0> and |1> state for each qubit)

MichaelBroughton

Hey Doug, good first pass! Left a few comments, some big, some small. Overall I think the code might have a subtle math mistake and could be shortened/tightened up a little bit here and there. There's also a few minor nits.

MichaelBroughton · 2021-07-14T21:24:55Z

cirq-core/cirq/__init__.py

@@ -96,6 +96,7 @@

 from cirq.experiments import (
    estimate_single_qubit_readout_errors,
+    estimate_parallel_readout_errors,


I'm still not a huge fan of having these two seperate methods. Reason being is that estimate_single_qubit_readout_errors can still run "parallel operations" in the same way that estimate_parallel_readout_errors does (it just won't use randomized computational basis states). I do also appreciate the need for name consistency with our parallel_p00 and isolated_p11_error etc. So how about this:

rename this to estimate_single_qubit_readout_errors_parallel

In a seperate PR add estimate_single_qubit_readout_errors_isolated and deprecate estimate_single_qubit_readout_errors.

What do you think ?

Changed this to "estimate_correlated_single_qubit_readout_errors"

MichaelBroughton · 2021-07-14T21:26:28Z

cirq-core/cirq/experiments/parallel_readout_calibration.py

+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+


Nit: Missing module docstring.

MichaelBroughton · 2021-07-14T21:28:17Z

cirq-core/cirq/experiments/parallel_readout_calibration.py

+    bit_strings: Optional[List[int]] = None,
+) -> ReadoutExperimentResult:
+
+    """Estimate readout error for qubits simultaneously.


Nit: "Estimate single qubit readout error using parallel operations."

MichaelBroughton · 2021-07-14T21:33:41Z

cirq-core/cirq/experiments/parallel_readout_calibration.py

+    For each trial, prepare a bitstring of random |0〉and |1〉states for
+    each state.  Measure each qubit.  Capture the errors per qubit of
+    zero and one state over each triel.


This is a little hard to digest for me. What do you think of ?

"For each trial, prepare and then measure a random computational basis bitstring on qubits using parallel gates. Returns a SingleQubitReadoutCalibrationResult which can be used to compute readout errors for each qubit in qubits as well as other quantities." ?

triel -> trial

MichaelBroughton · 2021-07-14T21:35:24Z

cirq-core/cirq/experiments/parallel_readout_calibration.py

+    zero and one state over each triel.
+
+    Args:
+        sampler: The quantum engine or simulator to run the circuits.


Nit: "The cirq.Sampler that will be used to run the readout characterization circuits." So that the docs infra can pickup the link to cirq.Sampler.

MichaelBroughton · 2021-07-14T22:15:54Z

cirq-core/cirq/experiments/parallel_readout_calibration.py

+    if trials_per_batch is not None:
+        if trials_per_batch <= 0:
+            raise ValueError("Must provide non-zero trials_per_batch for readout calibration.")
+        num_batchs = trials // trials_per_batch
+        if trials % trials_per_batch > 0:
+            num_batchs += 1
+    else:
+        num_batchs = 1
+        trials_per_batch = trials


What do we think of this slight reworking ? It's a little easier to digest I think.

if trials_per_batch is None: trials_per_batch = trials if trials_per_batch <= 0: raise ValueError("Must provide non-zero trials_per_batch for readout calibration.") num_batches = (trials + trials_per_batch - 1) // trials_per_batch

MichaelBroughton · 2021-07-14T22:29:50Z

cirq-core/cirq/experiments/parallel_readout_calibration.py

+    zero_state_trials: Dict[cirq.Qid, List[float]] = {q: [] for q in qubits}
+    one_state_trials: Dict[cirq.Qid, List[float]] = {q: [] for q in qubits}
+    for batch_result in results:
+        for trial_idx, trial_result in enumerate(batch_result):
+            for idx, q in enumerate(qubits):
+                had_x_gate = (bit_strings[idx] >> trial_idx) & 1
+                if had_x_gate:
+                    one_state_trials[q].append(1 - np.mean(trial_result.measurements[repr(q)]))
+                else:
+                    zero_state_trials[q].append(np.mean(trial_result.measurements[repr(q)]))
+
+    zero_state_errors = {q: np.mean(zero_state_trials[q]) for q in qubits}
+    one_state_errors = {q: np.mean(one_state_trials[q]) for q in qubits}


I'm worried this math might have some wiggle room for errors here. If I break up some batches into the following sizes:
trials = 22
num_batches = 3
and batch_sizes of [10, 10, 2]
Would this code then weight each of the batch sizes equally and increase the weight of the final 2 trial run disproportionately to the other 10 trial runs. Am I missing something here ?

This is a good catch! I think it may have been okay since the repetitions are the same for each trial, but I have fixed this to count up the trials anyway, since I think the previous code was error prone if not outright broken.

MichaelBroughton · 2021-07-14T22:44:21Z

cirq-core/cirq/experiments/parallel_readout_calibration.py

+    for batch in range(num_batchs):
+        circuit = circuits.Circuit()
+        single_sweeps = []
+        for idx, q in enumerate(qubits):
+            sym_val = f'bit_{idx}'
+            current_trial = bit_strings[idx]
+            trial_range = range(batch * trials_per_batch, (batch + 1) * trials_per_batch)
+            circuit.append(ops.X(q) ** sympy.Symbol(sym_val))
+            single_sweeps.append(
+                study.Points(
+                    key=sym_val, points=[(current_trial >> bit) & 1 for bit in trial_range]
+                )
+            )
+
+        circuit.append(ops.measure_each(*qubits, key_func=repr))
+        total_sweeps = study.Zip(*single_sweeps)
+        all_circuits.append(circuit)
+        all_sweeps.append(total_sweeps)


This is more of a stylistic choice here so feel free to ignore, what do you think of seperating things out a little bit to something like (untested):

flip_symbols = sympy.symbols(f'flip_0:{len(qubits)}') flip_circuit = cirq.Circuit([cirq.X(q) ** s for q, s in zip(qubits, flip_symbols)], [ops.measure_each(*qubits, key_func=repr)]) circuits = [flip_circuit] * num_batches for batch in range(num_batches): # single_sweeps = [] # for i, current_symbol in enumerate(flip_symbols): # current_sweep = study.Points(key=current_symbol, points=flip_values[batch][i]) # single_sweeps.append(ref_sweep) single_sweeps = [study.Points(key=symb, points=flip_values[batch][i]) for i, symb in enumerate(flip_symbols)] total_sweeps = study.Zip(*single_sweeps) all_sweeps.append(total_sweeps)

Where flip_values is a 3d numpy boolean array built from bit_strings, who's first dimension indexes batch, second dimension indexes qubit and third dimension indexes flip value (T/F) ?

Basically did this, but kept it as a 2d array without the batch dimension.

cirq-core/cirq/experiments/readout_experiment_result.py

MichaelBroughton · 2021-07-14T22:46:51Z

cirq-core/cirq/experiments/single_qubit_readout_calibration.py

+@deprecated(
+    deadline="v0.13",
+    fix="use cirq.experiments.ReadoutExperimentResult instead",
+    name="cirq.experiments.SingleQubitReadoutCalibrationResult",
+)
+class SingleQubitReadoutCalibrationResult(ReadoutExperimentResult):


From our discussion offline, we didn't think there was a compelling enough reason to want to rename this class with no major changes to existing properties or functionality. So this is fine to leave for now and maybe update in a future PR.

Reverted change.

mpharrigan · 2021-07-15T00:12:52Z

cirq-core/cirq/experiments/parallel_readout_calibration.py

+        trials: The number of bitstrings to prepare.
+        trials_per_batch:  If provided, split the experiment into batches
+            with this number of trials in each batch.
+        bit_strings: A list of ints that specifies the bit strings for each


very strange that these are raveled into a 1d list, and backwards to what I would have expected. Why not keep as a 2d array?

Changed to 2d numpy array.

MichaelBroughton

Just a few more nits/questions, then we'll be good to merge.

MichaelBroughton · 2021-08-09T17:50:36Z

cirq-core/cirq/experiments/__init__.py

@@ -72,6 +72,7 @@
 )

 from cirq.experiments.single_qubit_readout_calibration import (
+    estimate_correlated_single_qubit_readout_errors,


Why the name change here ? We aren't really estimating the correlations between single qubits.

Changed back to parallel

MichaelBroughton · 2021-08-09T17:50:49Z

cirq-core/cirq/experiments/single_qubit_readout_calibration.py

+"""Module for supporting single qubit readout experiments using
+either correlated or uncorrelated readout statistics.
+"""


Nit: multi line module docstring.

MichaelBroughton · 2021-08-09T17:55:02Z

cirq-core/cirq/experiments/single_qubit_readout_calibration.py

+    if repetitions <= 0:
+        raise ValueError("Must provide non-zero repetition for readout calibration.")
+    if bit_strings is None:
+        bit_strings = np.array([[random.randint(0, 1) for _ in range(trials)] for _ in qubits])


Nit: np.random.randint(0, 2, size=(len(qubits), trials))

Cool. Done.

MichaelBroughton · 2021-08-09T17:55:56Z

cirq-core/cirq/experiments/single_qubit_readout_calibration.py

+    if trials_per_batch <= 0:
+        raise ValueError("Must provide non-zero trials_per_batch for readout calibration.")
+
+    all_circuits = []


Nit: not needed.

MichaelBroughton · 2021-08-09T18:42:06Z

cirq-core/cirq/experiments/single_qubit_readout_calibration.py

+        bit_strings: Optional numpy array of shape (qubits, trials) where the
+            first dimension is the qubit (ordered by the qubit order from


Nit: Wouldn't it make more sense to reverse the dimensions here ? That way If the shape is [1, 9] I know I have one trial on nine qubits and it's easy to unpack each bitstring with a simple for bit_string in bit_strings: (See below)

cirq-core/cirq/experiments/single_qubit_readout_calibration.py

MichaelBroughton · 2021-08-09T19:01:15Z

cirq-core/cirq/experiments/single_qubit_readout_calibration.py

+        raise ValueError(
+            'bit_strings must be numpy array '
+            f'of shape (qubits, trials) ({len(qubits)}, {trials}) '
+            f"but was {bit_strings.shape if hasattr(bit_strings, 'shape') else None}"
+        )


Do we care about also checking for invalid bitstrings ? Right now I could pass [[47], [23]]

Added check.

MichaelBroughton · 2021-08-09T19:06:16Z

cirq-core/cirq/experiments/single_qubit_readout_calibration_test.py

@@ -67,7 +77,7 @@ def test_estimate_single_qubit_readout_errors_no_noise():
 def test_estimate_single_qubit_readout_errors_with_noise():
    qubits = cirq.LineQubit.range(5)
    sampler = NoisySingleQubitReadoutSampler(p0=0.1, p1=0.2, seed=1234)
-    repetitions = 1000
+    repetitions = 2000


Why the bump here ?

MichaelBroughton · 2021-08-09T19:13:45Z

cirq-core/cirq/experiments/single_qubit_readout_calibration.py

+        repetitions: The number of measurement repetitions to perform for
+            each trial.
+        trials: The number of bitstrings to prepare.
+        trials_per_batch:  If provided, split the experiment into batches


One other random question: Why is trials_per_batch something we want at the experiment level instead of allowing the user to construct a custom cirq.Sampler where they can make arbitrary calls to run_batch and then behind the scenes it get's broken up into chunks by the sampler ? This feels like a strange place to have something like this am I missing something ?

Fair enough, but we don't actually have this sampler right now. The other thing is that this is a sweep (not a batch), so it's a bit harder to break up. We could deprecate this parameter once we have a proper batching sampler maybe?

dstrain115 · 2021-08-09T20:28:06Z

Fixed most of the review comments, but wanted to take a moment to think about the numpy transposition since I am not totally sure which is better.

mpharrigan · 2021-08-09T20:47:33Z

re: numpy transposition. If you cirq.measure(*qubits, key='z') the resulting results.measurements['z'] has the "qubit axis" as the second dimension

MichaelBroughton

LGTM

MichaelBroughton · 2021-08-10T18:00:13Z

cirq-core/cirq/experiments/single_qubit_readout_calibration.py

+        raise ValueError("Must provide non-zero repetition for readout calibration.")
+    if bit_strings is None:
+        bit_strings = np.random.randint(0, 2, size=(trials, len(qubits)))
+    else:


Nit: else not needed.

I think it is worth keeping in, since it avoids a scan of the bit_string array when it is not passed in. This probably has a slight performance gain. We don't need to do the sanity checks if we generate the bit_string array.

This reverts commit eb72fb5.

Reverts #4307 which has introduced CI flakes here here: https://github.com/quantumlib/Cirq/pull/4402/checks?check_run_id=3297425897 Fixes #4404.

Adds an experiment for simultaneous (parallel) readout. This experiment (estimate_parallel_single_qubit_readout_errors) initializes a number of bitstrings(trials) and initializes the qubits to those bitstrings and immediately measures them It then estimates zero state and one state errors averaged over all of the trials where the given qubit was initialized to |0> or |1> respectively. This change also collapses the isolated single qubit readout experiment to use this function as well.

Reverts quantumlib#4307 which has introduced CI flakes here here: https://github.com/quantumlib/Cirq/pull/4402/checks?check_run_id=3297425897 Fixes quantumlib#4404.

dstrain115 added 2 commits July 7, 2021 15:54

Initial commit for simul readout characterization

82f5827

Refactor

d64fea4

dstrain115 requested review from cduck, mrwojtek, vtomole and a team as code owners July 9, 2021 20:38

dstrain115 requested a review from viathor July 9, 2021 20:38

google-cla bot added the cla: yes Makes googlebot stop complaining. label Jul 9, 2021

dstrain115 requested a review from karlunho July 9, 2021 20:38

MichaelBroughton reviewed Jul 12, 2021

View reviewed changes

Refactor single-qubit calibration into parallel calibration.

066d785

dstrain115 added 3 commits July 12, 2021 18:01

Add validation and tests.

fc56f35

Reorder since bits cannot be zero in python 3.8 and before.

fc4c38f

Fix tests and add tests for nan case.

ce1a938

MichaelBroughton requested changes Jul 14, 2021

View reviewed changes

mpharrigan reviewed Jul 15, 2021

View reviewed changes

balopat assigned MichaelBroughton Jul 20, 2021

dstrain115 added 5 commits August 3, 2021 09:04

Move things back into one file.

af953aa

Handle most of the review comments.

682d361

Re-add missing repr file.

f8ab64a

Change bit_strings into numpy array.

14b36d2

Remove dup docs.

e06bd87

dstrain115 requested review from mpharrigan and MichaelBroughton August 9, 2021 12:59

MichaelBroughton requested changes Aug 9, 2021

View reviewed changes

MichaelBroughton reviewed Aug 9, 2021

View reviewed changes

dstrain115 added 2 commits August 9, 2021 13:26

Most of mbbrough review comments except numpy transposition.

908f09a

Merge branch 'master' into simultaneous_readout

29a42fa

dstrain115 added 2 commits August 10, 2021 06:03

Transpose numpy and add error checking.

5e8d6e5

Merge branch 'master' into simultaneous_readout

86f4e06

dstrain115 requested a review from MichaelBroughton August 10, 2021 13:04

MichaelBroughton approved these changes Aug 10, 2021

View reviewed changes

dstrain115 merged commit eb72fb5 into quantumlib:master Aug 10, 2021

MichaelBroughton added a commit that referenced this pull request Aug 11, 2021

Revert "Simultaneous readout (#4307)"

71b86fe

This reverts commit eb72fb5.

MichaelBroughton mentioned this pull request Aug 11, 2021

Revert "Simultaneous readout" #4406

Merged

CirqBot pushed a commit that referenced this pull request Aug 11, 2021

Revert "Simultaneous readout" (#4406)

abc7159

Reverts #4307 which has introduced CI flakes here here: https://github.com/quantumlib/Cirq/pull/4402/checks?check_run_id=3297425897 Fixes #4404.

		bit_strings: Optional numpy array of shape (qubits, trials) where the
		first dimension is the qubit (ordered by the qubit order from

Simultaneous readout #4307

Simultaneous readout #4307

Conversation

dstrain115 commented Jul 9, 2021

MichaelBroughton left a comment

Choose a reason for hiding this comment

dstrain115 commented Jul 12, 2021

MichaelBroughton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelBroughton Jul 14, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelBroughton Jul 14, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelBroughton Jul 14, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelBroughton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dstrain115 commented Aug 9, 2021

mpharrigan commented Aug 9, 2021

MichaelBroughton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelBroughton Jul 14, 2021 •

edited

MichaelBroughton Jul 14, 2021 •

edited

MichaelBroughton Jul 14, 2021 •

edited