Add Mirror RB experiment by paco-ri · Pull Request #842 · qiskit-community/qiskit-experiments

paco-ri · 2022-07-11T19:04:38Z

Summary

Added mirror randomized benchmarking experiment and analysis classes

Details and comments

TODO

Accommodate mirror circuits of zero length

gadial

Generally looks great, there are some small issues that needs addressing.

gadial · 2022-07-12T09:13:23Z

+        if self._inverting_pauli_layer:
+            for circuit in circuits:
+                target = circuit.metadata["target"]
+                label = "".join(["X" if char == "1" else "I" for char in target])


I have to admit I don't understand what's going on here.

Added comments, please let us know if it makes sense now

…ircuits, comment pauli label

nkanazawa1989 · 2022-07-27T18:00:19Z

+illustrate, consider the two circuits below, both of which were generated in 
+``pyGSTi``. The first circuit was transpiled in ``pyGSTi``, 
+
+.. image:: pygsti-data-pygsti-transpiled-circ.png


Why this cannot be generated here? You have hidden cell option.

pyGSTi is required to generate these circuits, and we don't want to require pyGSTi to accurately render the documentation.

nkanazawa1989 · 2022-07-27T18:24:43Z

+        """
+
+        # Backend must have a coupling map
+        if not self._backend or not self._backend.configuration().coupling_map:


Hmm... I understand why we need this check here. Basically MirrorRB can target more than 2Q, then we should avoid applying 2Q gate to non-adjacent qubits otherwise extra SWAP is required.

On the other hand, the standard execution model of QE for now is

define circuit in the virtual qubit domain

call transpiler with initial layout, which is based on the experiment's physical qubits

run circuit in the physical qubit domain (transpiled circuit) on the target backend.

and this experiment intentionally avoids step 1 due to above constraint. This probably motivate us to turn the backend in the experiment class constructor into required (currently it's optional), and to deprecate another backend argument in the BaseExperiment.run method. This model indicates an experiment instance is always aware of the target backend, and we can reasonably generate circuits on top of some hardware architecture constraints -- as you are doing in this PR.

I see, the reason why we added this check instead of making the backend required is because @chriseclectic and @coruscating had mentioned that backends aren't serializable, so it may be better to keep it optional and only check for it when the circuits are being generated. Considering these points, what would be the best option here?

I lean toward having backend in the constructor as required because this benefits other experiments as well (e.g. T1 experiment needs to check timing constraints to build circuits). I think we should add special handling for backend in serialization/deserialization (I think it's enough to keep backend name instead of serialized object and ask user to set identical backend if the experiment is loaded instance).

We discussed several potential plans for handling this in the meeting. @nkanazawa1989 are you okay with keeping the current implementation for this experiment and merging first, and then working on changing the implementation in a future PR?

nkanazawa1989 · 2022-07-27T18:28:34Z

+        # Backend must have a coupling map
+        if not self._backend or not self._backend.configuration().coupling_map:
+            raise QiskitError(
+                "Must provide a backend with a coupling map or provide "


If a coupling map is not provided, you can generate all possible combination instead. Such data can be easily generated with itertools.

nkanazawa1989 · 2022-07-27T18:30:42Z

+
+        # Store transpiled circuits in metadata
+        for circ in transpiled:
+            circ.metadata["transpiled_qiskit_circ"] = circ


This will break QPY serialization I guess unless you provide custom Json serializer.

We've moved this to a PR in the internal repository

nkanazawa1989 · 2022-07-27T18:43:56Z

+        full_sampling: bool = False,
+        inverting_pauli_layer: bool = False,
+    ):
+        """Initialize a mirror randomized benchmarking experiment that uses


What is the benefit of this class for users? If this class is just for validation, you should extract the circuit generation logic and use it only in the unittest to compare the output circuits. If PyGSTi gives you something like x10 performance improvement, perhaps it's worth having it here, but not as a subclass. You can refer to the implementation of tomography module. There are several fitters available (numpy, scipy, cvxopt) but these are the single class and the internal logic is dynamically switched based on the analysis option. Something like this would work.

We plan to make a separate PR for moving this class to the internal repository.

coruscating · 2022-08-19T15:24:50Z

Would it be better to report error per layer instead of EPC? Especially since there will be non-Clifford and Clifford versions.

albertzhu01 · 2022-08-22T17:26:35Z

@coruscating Yeah that would be more accurate

CLAassistant · 2022-08-31T21:46:31Z

All committers have signed the CLA.

coruscating · 2022-09-08T18:08:18Z

I think this can be merged after addressing Naoki's comments and changing EPC to EPL. If issues come up later, those can be addressed in future PRs.

paco-ri · 2022-09-13T04:43:49Z

The term "EPC" is currently used in standard and interleaved RB, so it would have to be replaced in those implementations too. Or maybe it would be better to replace "EPC" with "EPL" in the PR with universal-gate-set MRB. This PR implements MRB with circuits generated by the "old" sampling method which, to my knowledge, has only been implemented with Clifford gates.

coruscating · 2022-09-14T14:49:07Z

Agreed, let's leave it as EPC for this PR then.

…release note

…n tests

…tream main

merav-aharoni · 2022-11-13T11:01:59Z

Assuming the branch https://github.com/Qiskit/qiskit-experiments/tree/feature/rb_speedup is merged before this PR, note two things:

The methods CliffordUtils::random_clifford_circuits and CliffordUtils::random_cliffords will be deprecated. Use StandardRB::__sample_sequence instead.
Consider whether using the mapping of Cliffords to integers is worthwhile for computing the inverses. See CliffordUtils::_CLIFFORD_INVERSE_1Q

ihincks

I am very happy to see this PR as I think Mirror RB will a great asset to experiments!

My biggest concern with this PR is that MirrorRB, as implemented, doesn't let users specify the layers to use or the distribution over them, which is arguably the biggest selling point of mirror RB. Instead, the layers and the distribution are chosen by random_edgegrab_clifford_circuits. This assumption is baked into the API, so that generalizing MirrorRB to more diverse distributions would likely require a breaking change. To be clear, my main concern is the API; I think that using random edges is a very good thing to include in a first release, and also a very sensible default.

I suggest looking into whether the distribution itself could be an argument to the experiment, with random_edgegrab_clifford_circuits being one possible value. The distribution could be a callable returning layers, or something.

I have left a few other comments which, relative to the above, I view as being minor.

Among these comments, I'd just like to highlight that the experiment in a few places is described as being "more scalable" and also that "it can be used to detect crosstalk errors". I think the former statement is often worded to not give enough credit to other scalable characterization protocols that came before it, though I realize that (I think) the intent is to compare it only with standard randomized benchmarking using huge Cliffords. The latter statement, while true, I think is a bit narrow to be emphasized so heavily. Mirror RB at its core is for estimating fidelities marginalized over distributions of layers, and using several experiments to infer information about crosstalk is just one particular application of this ability.

ihincks · 2022-11-07T13:49:22Z

        .. ref_website:: qiskit-experiments, https://github.com/Qiskit/qiskit-experiments

    """
+


The changes to this file look unnecessary; please revert them if so.

ihincks · 2022-11-07T14:00:40Z

+    A new experiment class :class:`qiskit_experiments.library.MirrorRB` is 
+    introduced. This class implements mirror randomized benchmarking (RB), a version 
+    of RB that uses mirror circuits. It is more scalable than other RB protocols and
+    can consequently be used to detect crosstalk errors. 


There are many scalable RB protocols (direct fidelity estimation, cycle benchmarking, xeb, etc), so I think we sholud be a bit careful about the blanket statement "It is more scalable than other RB protocols". I suggest:

A new experiment class :class:`qiskit_experiments.library.MirrorRB` is introduced. This class implements mirror randomized benchmarking (RB), which is a protocol that measures the fidelity of user-defined ensembles of randomized mirror circuits.

ihincks · 2022-11-07T19:21:20Z

@@ -41,8 +41,8 @@
   "outputs": [],
   "source": [
    "IBMQ.load_account()\n",


The changes to this file look unnecessary; please revert them if so.

ihincks · 2022-11-07T19:21:27Z

@@ -40,8 +40,8 @@
   "outputs": [],
   "source": [
    "IBMQ.load_account()\n",


The changes to this file look unnecessary; please revert them if so.

ihincks · 2022-11-22T18:08:43Z

+        Args:
+            qubits: A list of physical qubits for the experiment.
+            lengths: A list of RB sequences lengths.
+            local_clifford: If True, begin the circuit with uniformly random 1-qubit


What value do the optional "local_clifford" and "pauli_randomize" bring to this experiment?

ihincks · 2022-11-22T18:39:00Z

+Mirror RB experiment
+--------------------
+
+Mirror RB is a RB protocol that is more scalable to larger numbers of qubits, 


I think that "more scalable" needs to be qualified a bit, and that crosstalk detection needs to be discussed as an application of the protocol rather than a central aspect. Indeed, standard randomized benchmarking in parallel can do some types of crosstalk detection.

albertzhu01 added 2 commits July 11, 2022 14:26

add mirror RB experiment and analysis files

b493b29

edit appropriate mirrorRB-relevant files for PR

11b8af2

coruscating requested changes Jul 11, 2022

View reviewed changes

coruscating reviewed Jul 11, 2022

View reviewed changes

Comment thread test/randomized_benchmarking/test_randomized_benchmarking.py Outdated

coruscating added the Changelog: New Feature Include in the "Added" section of the changelog label Jul 11, 2022

albertzhu01 added 2 commits July 11, 2022 17:01

update docstrings

f3121eb

delete commented series code

b4e6072

coruscating assigned albertzhu01 Jul 11, 2022

gadial self-requested a review July 12, 2022 05:51

gadial suggested changes Jul 12, 2022

View reviewed changes

albertzhu01 and others added 10 commits July 12, 2022 09:11

fix pygsti version

aef0379

remove outcomes setting

933bc7b

remove prints and add paper ref

094ebd0

remove prints and add paper ref

0e1ec4d

move coupling map outside loop, delete elements_without_inv and int_c…

f28cdb7

…ircuits, comment pauli label

Merge branch 'main' of https://github.com/albertzhu01/qiskit-experiments

281b777

reformat files with black and lint

9a4602a

cleaned RB tutorial

1c9b350

cleaned RB tutorial

66c6f3e

moved Clifford sampling in edgegrab algo

374fc2e

nkanazawa1989 reviewed Jul 27, 2022

View reviewed changes

use full connectivity for coupling map if coupling map is not provided

34e2cb7

paco-ri force-pushed the main branch from 36301ab to 34e2cb7 Compare August 31, 2022 22:17

paco-ri added 4 commits August 31, 2022 18:23

resolved some merge conflits

fefc4ba

changed AnalysisResult imports

b8ab48d

edited BitGenerator and SeedSequence imports

5cd64e5

removed MirrorRBPyGSTi from library init file

b43325a

albertzhu01 added 5 commits October 30, 2022 20:49

fix full connectivity map generation code

f172c9b

remove MirrorRBPyGSTi from mirror_rb_experiment.py, rb tutorial, and …

d0feba2

…release note

fix oupling map full connectivity generation and update aer imports i…

d069f5d

…n tests

fix merge conflicts with upstream main branch

6846e12

reformat imports (temporarily) to pass lint with merged code from ups…

ddfcbaf

…tream main

ihincks reviewed Nov 22, 2022

View reviewed changes

merged main

1124ea8

coruscating added this to the Release 0.5 milestone Dec 13, 2022

merged main branch

057de84

coruscating mentioned this pull request Mar 17, 2023

Clifford Mirror RB experiment #1090

Open

coruscating removed this from the Release 0.5 milestone Mar 28, 2023

		.. ref_website:: qiskit-experiments, https://github.com/Qiskit/qiskit-experiments

		"""

Uh oh!

Conversation

paco-ri commented Jul 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details and comments

TODO

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gadial left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coruscating commented Aug 19, 2022

Uh oh!

albertzhu01 commented Aug 22, 2022

Uh oh!

CLAassistant commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coruscating commented Sep 8, 2022

Uh oh!

paco-ri commented Sep 13, 2022

Uh oh!

coruscating commented Sep 14, 2022

Uh oh!

merav-aharoni commented Nov 13, 2022

Uh oh!

ihincks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paco-ri commented Jul 11, 2022 •

edited

Loading

CLAassistant commented Aug 31, 2022 •

edited

Loading