Skip to content

Allow to pass Circuit to single-qubit gates compensation #4118

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

Merged
merged 8 commits into from
Jun 9, 2021

Conversation

mrwojtek
Copy link
Collaborator

@mrwojtek mrwojtek commented May 18, 2021

The current workflow of calibrations is first to call prepare_characterization_for_moments which returns the annotated Circuit wrapped in an instance of CircuitWithCalibration. Although this solution is efficient because allows to extract moments to characterize only once (which is not a cheap operation), this is not convenient to use in practice. In the current situation this annotated circuit must be stored and passed to make_zeta_chi_gamma_compensation_for_moments later.

This change allows to pass Circuit into make_zeta_chi_gamma_compensation_for_moments which will do the same analysis as was done before again. However, it makes the API more convenient to use.

@mrwojtek mrwojtek requested review from cduck, vtomole and a team as code owners May 18, 2021 13:42
@mrwojtek mrwojtek requested a review from 95-martin-orion May 18, 2021 13:42
@google-cla google-cla bot added the cla: yes Makes googlebot stop complaining. label May 18, 2021
@mrwojtek mrwojtek requested a review from mpharrigan May 18, 2021 13:48
Copy link
Collaborator

@mpharrigan mpharrigan left a comment

Choose a reason for hiding this comment

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

Couple comments.

This is technically a breaking change since you're changing the name of an argument. It's probably fine though. I bet most people would be using it as a positional arg

@@ -834,6 +853,42 @@ def make_zeta_chi_gamma_compensation_for_moments(
The moment to calibration mapping is updated for the new circuit so that successive
calibrations could be applied.
"""

if isinstance(circuit, Circuit):
Copy link
Collaborator

Choose a reason for hiding this comment

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

factor this out into a helper method?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, I wanted to send out this PR quickly so it wasn't really clean.

Comment on lines 838 to 839
match the circuit against a given characterizations. This step is not computationally
efficient and can be avoided by passing the pre-calculated instance of
Copy link
Collaborator

Choose a reason for hiding this comment

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

what do you mean by not efficient? Am I correct in deducing that it has a quadratic runtime where it scans over all the calibrations for each moment? Is this bad in practice?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have changed the docstring.

In the beginning I was fixed on optimizing this computational cost but after looking at this right now it's probably not relevant. This code will not scale to large circuits and more quibts anyway so it's probably fine to do this calculations many times for now. It would probably be also cleaner in terms of API if we did all the compilation like that from the start: this allows to mix different characterizations (XEB/Floquet together as well) and the structure "CircuitsWithCalibrations" would not be necessary.

@mrwojtek
Copy link
Collaborator Author

PTAL

@balopat balopat requested a review from mpharrigan June 8, 2021 00:24
@mrwojtek mrwojtek requested a review from wcourtney as a code owner June 9, 2021 07:26
@mrwojtek mrwojtek merged commit 2d7641b into quantumlib:master Jun 9, 2021
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
…4118)

* Allow to pass Circuit to Z gates compensation

* Apply review comments

* Fix linter errors

* Fix linter errors

* Fix missing coverage

* Fixes after merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants