Mirror circuit fidelity estimation support and introduction of benchmarking interface (name TBD) #628

ndsieki · 2025-08-05T16:36:18Z

This PR adds a benchmark creation tool we are calling B-Sqale (for the time being), integrates mirror circuit benchmarking into pyGSTi, and improves Qiskit interfacing.

Circuit mirroring functionality is supported by:

A RandomCompilation class, which OOP-ifies both the central Pauli (https://www.nature.com/articles/s41567-021-01409-7) and random compilation (https://arxiv.org/abs/2204.07568) mirroring routines.
Circuit mirroring functions in pygsti.protocols.mirror_edesign. The core routine is make_mirror_edesign, which creates a CombinedExperimentDesign whose circuits can be executed to perform mirror circuit fidelity estimation (MCFE). There are three other functions - qiskit_circuits_to_mirror_edesign, qiskit_circuits_to_fullstack_mirror_edesign, and qiskit_circuits_to_subcircuit_mirror_edesign - which build on make_mirror_edesign to create different kinds of benchmarks directly from Qiskit circuits.
An added from_mirror_experiment class method for the VBDataFrame class, which streamlines the analysis process for MCFE. The VBDataFrame class also now includes plotting functionality for creating volumetric benchmarking plots from subcircuit mirror experiments.

This PR also adds B-Sqale (name subject to change), a scalable and robust benchmark creation tool, to pyGSTi. More information will be available in a forthcoming paper. The submodule pygsti.tools.bsqale provides wrappers for the mirror benchmarking functionality:

noise_mirror_benchmark (wrapper for qiskit_circuits_to_mirror_edesign)
fullstack_mirror_benchmark (wrapper for qiskit_circuits_to_fullstack_mirror_edesign)
subcircuit_mirror_benchmark (wrapper for qiskit_circuits_to_subcircuit_mirror_edesign)
calculate_mirror_benchmark_results (wrapper for VBDataFrame.from_mirror_experiment)

These wrappers are provided in anticipation of expanding B-Sqale to scalable benchmarking techniques beyond MCFE.

Interfacing with Qiskit is supported by the addition of a from_qiskit class method and convert_to_qiskit method to the Circuit class.

…MCFE utilities

…pytket import since it is no longer needed

… uses a more reasonable test-ref match verification

… vanilla MCFE

…re reference circuit does not use ancillae, other changes

…ration

coreyostrove · 2025-08-13T06:04:38Z

Quick note: I expect to be able to merge in #379 tomorrow morning. Once that is on develop I'll ping you and then let's merge develop into your branch to use as the baseline for reviewing. This will knock out a decent number of changes and reduce any redundancy.

coreyostrove · 2025-08-13T16:18:33Z

Update: #379 is now merged into develop. It looks like there might be a couple conflicts to resolve when merging develop into your branch, so hopefully those won't be tricky. I'll start my review once that merge is done.

…ration

…ritance

ndsieki · 2025-08-13T17:08:33Z

test/performance/mpi_2D_scaling/mpi_timings.py

This file was identical to run_me_with_mpirun.py.

ndsieki · 2025-08-13T17:10:03Z

Merged in develop and fixed a couple of minor things. @coreyostrove, should be ready for your review.

…t sample in the same width/depth

coreyostrove

This is a true tour-de-force, thanks for the massive amount of effort that went into the development of these new capabilities, @ndsieki!

I have made a few suggestions and requests inline. Nothing that constituted a major concern as I recall (your set of unit tests looked rather comprehensive, which goes a long way to assuaging any concerns), and hopefully these should be fairly quick to enact. Please see in line comments, but a couple high-level recurring themes:

Please take a pass through the new code and give it a spring cleaning. I noticed a large number of commented out print statements (presumably from debugging and development), older/obsolete implementations, etc. If you could identify the stragglers which are no longer needed that'd be great.
Move try-except blocks into relevant functions, specifically for the qiskit and matplotlib imports. The rationale here is that not everyone will have qiskit installed, and most of the time that is fine, we should only raise warnings when touching functionality that requires it.

Also, we'll want to make sure all of your unit tests are passing. As of last night they weren't but it looks like you recently pushed some changes which may have addressed those.

Overall this all looks good though. Sorry it took a while to review (there was a lot of code here, in my defense). Ping me when you've had a chance to implement these changes and I strongly suspect we'll be good to go at that point to fully approve and merge this in.

coreyostrove · 2025-08-14T04:01:41Z

jupyter_notebooks/Examples/MCFE-Example.ipynb

It is hard to add this comment inline, but you have a cell in this example notebook under the heading "Compute process fidelity for each circuit" which contains a bunch of postprocessing functions which ultimately return a pandas dataframe with the estimate process fidelities. Is the functionality of these helper function implemented in pyGSTi proper? You mentioned in the PR notes a new method for VBDataFrame which sounded like it would superficially produce a comparable output to your helper functions.

coreyostrove · 2025-08-14T04:02:21Z

pygsti/baseobjs/label.py

@@ -1893,7 +1893,7 @@ def to_native(self):
        """
        return tuple(self)

-    def replacename(self, oldname, newname):
+    def replace_name(self, oldname, newname):


Good catch!

coreyostrove · 2025-08-14T04:10:04Z

pygsti/baseobjs/qubitgraph.py

@@ -878,7 +878,7 @@ def subgraph(self, nodes_to_keep, reset_nodes=False):
            qubit_labels = nodes_to_keep

        edges = []
-        for edge in self.edges():
+        for edge in self.edges(include_directions=True):


I agree, can you add this kwarg to subgraph (defaulted to True, for the reasons you've stated) and plumb this through?

coreyostrove · 2025-08-14T04:12:30Z

pygsti/circuits/circuit.py

+    if qiskit.__version__ != '2.1.1':
+        _warnings.warn("Circuit class method `from_qiskit()` and method `convert_to_qiskit`"
+                       "is designed for qiskit version 2.1.1 and may not \
+                        function properly for your qiskit version, which is " + qiskit.__version__)


We should revisit down the line whether to either revise this warning to include a broader range of acceptable versions as this gets tested with newer versions of qiskit, or if it is really important we're on a specific qiskit version lets pin that in the pyGSTi requirements.

coreyostrove · 2025-08-14T04:16:33Z

pygsti/circuits/circuit.py

@@ -22,6 +25,16 @@
 from pygsti.tools import slicetools as _slct
 from pygsti.tools.legacytools import deprecate as _deprecate_fn

+try:


Let's move the try except block for the qiskit import into the definitions of from_qiskit and convert_to_qiskit. And with that being done have the except block actually raise that ImportError (with your additional context information), as at that point the failure of the import is critical.

coreyostrove · 2025-08-14T18:56:02Z

pygsti/protocols/mirror_edesign.py

+        "'qiskit_circuits_to_fullstack_mirror_edesign', and" \
+        "'qiskit_circuits_to_subcircuit_mirror_edesign are designed for qiskit 2.1.1. Your version is " + qiskit.__version__)
+
+    from qiskit import transpile


Similar comment to the previous ones related to this try-except block. Let's move these into the qiskit_* functions, as the core make_mirror_edesign doesn't rely on this, and we don't want to raise warnings for everyone if they're not using the qiskit functionality.

coreyostrove · 2025-08-14T18:57:58Z

pygsti/protocols/vbdataframe.py

+    import matplotlib.pyplot as _plt
+    import matplotlib as _mp
+except:
+    _warnings.warn('matplotlib is required for dataframe plotting, and does not appear to be installed.')


Please move this into create_vb_plot

coreyostrove · 2025-08-14T19:05:13Z

pygsti/tools/bsqale.py

+Please see <paper link forthcoming> for more information.
+"""
+
+# TODO: add copyright assertion


Please add this.

coreyostrove · 2025-08-14T19:05:40Z

pygsti/tools/bsqale.py

+        "'qiskit_circuits_to_fullstack_mirror_edesign', and" \
+        "'qiskit_circuits_to_subcircuit_mirror_edesign are designed for qiskit 2.1.1. Your version is " + qiskit.__version__)
+    from qiskit import transpile
+


See previous comments on moving these try and excepts to the relevant functions reliant on qiskit.

coreyostrove · 2025-08-14T19:07:10Z

pygsti/tools/internalgates.py

+    if qiskit.__version__ != '2.1.1':
+        _warnings.warn("function 'standard_gatenames_qiskit_conversions()' is designed for qiskit version 2.1.1 \
+                and may not function properly for your qiskit version, which is " + qiskit.__version__)
+


See previous comments regarding the qiskit try-except blocks.

nkoskelo · 2025-08-14T21:00:59Z

pygsti/circuits/subcircuit_selection.py

+        if verbosity > 0:
+            print(f'Found subcircuit with {total_dropped_gates} dropped gates, {compiled_depth} depth, and {total_dangling_gates} dangling gates')
+
+    if (failures == 1000):


Should this be a variable global to the file which can be modified if the user is willing to wait longer?

This is a good suggestion. A standard practice in that case is to name it using all caps, and to give it an evocative name unlikely to result in namespace conflicts. something like MAX_STARTING_LAYER_ATTEMPTS.

…tions, remove debug commented code

…-string quotes

…ot imported

…ration

ndsieki · 2025-08-18T11:26:01Z

@coreyostrove, I've made the changes you requested (along with those suggested by @nkoskelo). Please let me know if I missed anything,

coreyostrove

Everything here looks good as far as I am concerned, thank you for your effort addressing all of the suggestions and requests. @tjproct is going to try to give this a quick once-over to try and catch any possible MCFE math-related bugs, which I am less well-suited to do on my end. Once we've gotten the thumbs-up from him I'll officially mark this as approved and we can get it merged in.

tjproct

I have only given the code a brief review due to time constraints right now. It looks good to me. I suspect that we may want to, in the future, rename some functions and do bits and pieces of minor restructuring / removing redundancy with other benchmarking code (e.g., I think there might be redundancy with data analysis code used for mirror RB). However, I think it makes sense to accept this now, and to update the code when we have time in the future (and prior to final publication of Noah's paper that's based around this code). In particular, I (personally) anticipate spending some time in the fall revamping the broader pyGSTi benchmarking code as part of developing some new benchmarks + merging in some hacked-together new benchmarking capabilities that currently exist on a feature branch. During that process it would likely make sense to make small tweaks to this code and remove redundancies at that point.

coreyostrove

@tjproct: That all sounds good to me!

@ndsieki: With the thumb's up from@tjproct we're good to go, thanks for your effort pushing this past the finish line!

ndsieki and others added 20 commits November 21, 2024 13:43

Create branch feature-mcfe-integration with preliminary inclusion of …

3093bc2

…MCFE utilities

Resolve utils circular import

d2b1137

remove outstanding unused utils import

88a267e

Refactor Pauli RC and central pauli into RandomCompilation class

b89d1e2

Modify RandomCompilation for testing, add U3 and CNOT unit tests

40b5711

add preliminary qiskit->pygsti circuit conversion utility and remove …

a48f2c2

…pytket import since it is no longer needed

Add random compilation and central pauli unit tests

57a7e67

Add from_qiskit method to Circuit class

515a7e8

Bugfixes to IBMQ interfacing and line ordering, add example notebook

7afd5e2

Subcirc selection now takes circ list instead of dict, mirror edesign…

a5e5750

… uses a more reasonable test-ref match verification

fix typo in mirror edesign test-ref circuit matching assertion

45bea5d

Update from_qiskit() method to handle Qiskit circ with multiple qregs

0d66a82

Factor MCFE VB process fidelity calculations into pyGSTi

57c7853

Preliminary functionizations of notebooks for fullstack, subcirc, and…

9ad4941

… vanilla MCFE

More fixes and updates for MCFE paper

b696de9

Clamp PF estimates, modify full-stack mirror edesign creation to ensu…

c91b04f

…re reference circuit does not use ancillae, other changes

add preliminary B-Sqale API

18cf81d

Merge remote-tracking branch 'origin/develop' into feature-mcfe-integ…

959468e

…ration

Add function documentation

343643e

Function renames and typo fixes

6e5d5b9

ndsieki requested review from tjproct and coreyostrove August 5, 2025 16:36

ndsieki self-assigned this Aug 5, 2025

ndsieki added this to the 0.9.14 milestone Aug 5, 2025

ndsieki added 6 commits August 5, 2025 11:22

Repair central pauli code, fix unit tests

c78453c

Remove inline ipdb debug comments

7ff81c6

Finish vbdataframe docstrings

5fc09e7

Add docstrings and remove outdated code paths

ce59e72

Add qiskit conversion unit tests

664436b

Add subcircuit selection unit tests

bd1428a

ndsieki marked this pull request as ready for review August 12, 2025 20:56

ndsieki requested review from rileyjmurray and a team as code owners August 12, 2025 20:56

ndsieki requested a review from adhumu August 12, 2025 20:56

ndsieki added 2 commits August 13, 2025 12:44

Merge remote-tracking branch 'origin/develop' into feature-mcfe-integ…

3816743

…ration

Remove duplicate ECR conversion and fix CombinedExperimentDesign inhe…

d6b6491

…ritance

ndsieki commented Aug 13, 2025

View reviewed changes

test/performance/mpi_2D_scaling/mpi_timings.py Outdated

Copy link

Contributor Author

ndsieki Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was identical to run_me_with_mpirun.py.

ndsieki added 3 commits August 13, 2025 20:12

Changes for Qiskit 2.1.1

919b398

Fix subcirc selection to sample a new qubit subset for each subcircui…

57239e6

…t sample in the same width/depth

Fix variable name overlap

bc723b4

coreyostrove requested changes Aug 14, 2025

View reviewed changes

nkoskelo reviewed Aug 14, 2025

View reviewed changes

Add copyright assertions, move import try-except statements into func…

9533e83

…tions, remove debug commented code

coreyostrove mentioned this pull request Aug 15, 2025

Randomized Compiling for General Gates #637

Open

ndsieki added 5 commits August 14, 2025 23:29

Make verbose print statements more explanatory

d248d98

Add GenericBackendV2 as a supported type for ibmq_backend and fix f…

2dbdd55

…-string quotes

Guard IBMQExperiment test Qiskit import and skip tests if Qiskit is n…

e418630

…ot imported

Merge remote-tracking branch 'origin/develop' into feature-mcfe-integ…

59ec70f

…ration

Remove stray comment and add missing copyright assertion

442d8b0

coreyostrove reviewed Aug 18, 2025

View reviewed changes

tjproct approved these changes Aug 19, 2025

View reviewed changes

coreyostrove approved these changes Aug 20, 2025

View reviewed changes

coreyostrove merged commit 9864c24 into develop Aug 20, 2025
4 checks passed

Mirror circuit fidelity estimation support and introduction of benchmarking interface (name TBD) #628

Mirror circuit fidelity estimation support and introduction of benchmarking interface (name TBD) #628

Uh oh!

Conversation

ndsieki commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coreyostrove commented Aug 13, 2025

Uh oh!

coreyostrove commented Aug 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ndsieki commented Aug 13, 2025

Uh oh!

coreyostrove 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!

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!

ndsieki commented Aug 18, 2025

Uh oh!

coreyostrove left a comment

Choose a reason for hiding this comment

Uh oh!

tjproct left a comment

Choose a reason for hiding this comment

Uh oh!

coreyostrove left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ndsieki commented Aug 5, 2025 •

edited

Loading