Skip to content
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

Combine RB and XEB to compute inferred errors #6455

Merged
merged 27 commits into from
Feb 13, 2024
Merged

Combine RB and XEB to compute inferred errors #6455

merged 27 commits into from
Feb 13, 2024

Conversation

NoureldinYosri
Copy link
Collaborator

@NoureldinYosri NoureldinYosri commented Feb 10, 2024

This PR:

  • Adds the option to specify the qubits to the XEB workflow
  • Adds pauli error computation for 2q XEB
  • Adds InferredXEBResult class which
    • computes inferred errors: pauli, XEB, and decay constant
    • has a convenience method to plot heatmap for each of the errors
    • has a convenience method to plot histogram of each of the erros.

@CirqBot CirqBot added the size: M 50< lines changed <250 label Feb 10, 2024
@NoureldinYosri NoureldinYosri marked this pull request as ready for review February 10, 2024 00:58
Copy link
Collaborator

@eliottrosenberg eliottrosenberg left a comment

Choose a reason for hiding this comment

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

LGTM, but it would be good if we could add a few things, either in this PR or the next one.

Maybe we could have a single function that the user calls which runs both 2Q XEB and 1Q RB and returns a CombinedXEBRBResult. The CombinedXEBRBResult would then need methods for returning and visualizing the 1Q error rates too. For the histogram method, we could plot 1Q and 2Q errors on the same axes (maybe something like result.histogram(single_qubit=True, two_qubit=True)).

Thanks!

cirq-core/cirq/experiments/two_qubit_xeb.py Outdated Show resolved Hide resolved
cirq-core/cirq/experiments/two_qubit_xeb.py Outdated Show resolved Hide resolved
cirq-core/cirq/experiments/two_qubit_xeb.py Show resolved Hide resolved
@NoureldinYosri
Copy link
Collaborator Author

@eliottrosenberg I added the ability to plot both single and two qubit pauli errors


The issue with a single function that runs both 2Q XEB and 1Q RB is the number of arguments which is not the best thing to have. That method will have 10+ arguments. alternatively I could group them into an RBOptions and XEBOptions classes, what do you think?

Copy link

codecov bot commented Feb 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c3de706) 97.82% compared to head (9f7d628) 97.82%.

❗ Current head 9f7d628 differs from pull request most recent head f14bf7e. Consider uploading reports for the commit f14bf7e to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #6455    +/-   ##
========================================
  Coverage   97.82%   97.82%            
========================================
  Files        1115     1115            
  Lines       97337    97448   +111     
========================================
+ Hits        95216    95327   +111     
  Misses       2121     2121            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@eliottrosenberg
Copy link
Collaborator

eliottrosenberg commented Feb 12, 2024

@eliottrosenberg I added the ability to plot both single and two qubit pauli errors

The issue with a single function that runs both 2Q XEB and 1Q RB is the number of arguments which is not the best thing to have. That method will have 10+ arguments. alternatively I could group them into an RBOptions and XEBOptions classes, what do you think?

I'm thinking that most of the parameters would be shared. It could be something like this:

Shared parameters:

  • sampler
  • qubits
  • repetitions
  • num_circuits
  • random_state (seed for rng)

RB:

  • num_clifford_range (call this depths_rb or something)

XEB:

  • entangling gate
  • cycle_depths (call this depths_xeb or something)

We can get rid of the use_xy_basis option for RB here, since we will always want that to be its default value, I think. So that's 8 parameters, and all of them will have default values except sampler, so it will be easy to use.

@NoureldinYosri
Copy link
Collaborator Author

@eliottrosenberg sg, I will add that method in a followup PR

Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

LGTM with some minor suggestions.

cirq-core/cirq/experiments/__init__.py Outdated Show resolved Hide resolved
cirq-core/cirq/experiments/two_qubit_xeb.py Outdated Show resolved Hide resolved
cirq-core/cirq/experiments/two_qubit_xeb.py Show resolved Hide resolved
cirq-core/cirq/experiments/two_qubit_xeb.py Outdated Show resolved Hide resolved
cirq-core/cirq/experiments/two_qubit_xeb.py Outdated Show resolved Hide resolved
cirq-core/cirq/experiments/two_qubit_xeb.py Outdated Show resolved Hide resolved
cirq-core/cirq/experiments/two_qubit_xeb.py Show resolved Hide resolved
cirq-core/cirq/experiments/two_qubit_xeb.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

LGTM with a final nit.

@NoureldinYosri NoureldinYosri enabled auto-merge (squash) February 13, 2024 23:01
@NoureldinYosri NoureldinYosri merged commit adf5155 into main Feb 13, 2024
34 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants