Skip to content

Expand Floquet calibration to arbitrary FSim gates #4164

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 39 commits into from
Jun 21, 2021

Conversation

fedimser
Copy link
Contributor

@fedimser fedimser commented Jun 5, 2021

  • Added a method try_convert_gate_to_fsim which can convert to PhaseCalibratedFSimGate wide range of gates, including sqrt(iSWAP), Sycamore and Controlled-Z gates.
  • Used this method (instead of try_convert_sqrt_iswap_to_fsim) as default gate translator in all methods in workflow.py.

As a result, compensation step Floquet calibration now works not only for sqrt(iSWAP), but also for:

  • all FSim gates (including Sycamore gate);
  • PhasedFSim gates (such that zeta=gamma=0);
  • all ISwapPow gates;
  • all PhasedISwapPow gates;
  • CZPow gates without global phase (including CZ).

This change doesn't affect characterization step the Floquet calibration.

@fedimser fedimser requested review from cduck, vtomole, wcourtney and a team as code owners June 5, 2021 18:19
@fedimser fedimser requested a review from balopat June 5, 2021 18:19
@google-cla google-cla bot added the cla: yes Makes googlebot stop complaining. label Jun 5, 2021
@fedimser fedimser marked this pull request as draft June 5, 2021 18:21
@fedimser fedimser marked this pull request as ready for review June 5, 2021 19:54
@mrwojtek mrwojtek self-requested a review June 7, 2021 07:15
Copy link
Collaborator

@mrwojtek mrwojtek left a comment

Choose a reason for hiding this comment

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

Thank you Dmytro for doing this! The concepts contained here are a little bit complex. I've tried to explain some of them but feel free to ping me if some of my comments are not clear for you.

@fedimser
Copy link
Contributor Author

fedimser commented Jun 8, 2021

Hi Wojtek, I resolved most of your comments, and rewrote try_convert_gate_to_fsim so it tries to make engine gate as "canonical" as possible, in paricular now theta is in range [0, pi).
I am still not sure how to write good integration test for the new compensation - let's discuss that. Should i expand engine simulator to support arbitrary imperfect FSim gates (and not only sqrt(iSWAP)).

@fedimser fedimser marked this pull request as draft June 10, 2021 12:46
@fedimser
Copy link
Contributor Author

fedimser commented Jun 14, 2021

I added a test utilizing the engine simulator. Please take another look.
@mrwojtek

@fedimser fedimser marked this pull request as ready for review June 14, 2021 11:00
@fedimser
Copy link
Contributor Author

Hi @mrwojtek, I addressed your comments and added more tests. PTAL.

Copy link
Collaborator

@mrwojtek mrwojtek left a comment

Choose a reason for hiding this comment

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

Three minor issues and one question which maybe you could clarify for me and we are done I think.

engine_simulator = cirq_google.PhasedFSimEngineSimulator.create_from_dictionary(
parameters={
(a, b): {
cirq.FSimGate(theta=0, phi=np.pi): params_cz_ab,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you use cirq.CZ gate here and below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't, because cirq.CZ != FSimGate(theta=0, phi=np.pi), but gate translator converts all gates to FSimGate. So all gates in this map must be FSimGate, otherwise there will be no match.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, you're right. If this code will be used heavily in the future we might think about normalizing all gates to a common form inside the "create_from_dictionary" by using the converter itself. (Let's postpone it for now.)

@fedimser
Copy link
Contributor Author

I addressed issue with global_phase and other suggestions.
@mrwojtek

engine_simulator = cirq_google.PhasedFSimEngineSimulator.create_from_dictionary(
parameters={
(a, b): {
cirq.FSimGate(theta=0, phi=np.pi): params_cz_ab,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, you're right. If this code will be used heavily in the future we might think about normalizing all gates to a common form inside the "create_from_dictionary" by using the converter itself. (Let's postpone it for now.)

@mrwojtek
Copy link
Collaborator

mrwojtek commented Jun 17, 2021

@balopat can we have approval from code owners for this?

Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

LGTM % comment

@@ -1038,3 +1046,51 @@ def try_convert_sqrt_iswap_to_fsim(gate: cirq.Gate) -> Optional[PhaseCalibratedF
return PhaseCalibratedFSimGate(cirq.FSimGate(theta=np.pi / 4, phi=0.0), 0.5)

return None


def try_convert_gate_to_fsim(gate: cirq.Gate) -> Optional[PhaseCalibratedFSimGate]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Unrelated comment] We should work towards getting cirq.match()! #2536

@balopat balopat added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jun 21, 2021
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jun 21, 2021
@CirqBot CirqBot merged commit a2f107c into quantumlib:master Jun 21, 2021
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jun 21, 2021
CirqBot pushed a commit that referenced this pull request Jun 24, 2021
Goal of this PR is that circuits having any of these 2-qubit gates (sqrt(iSWAP), Sycamore) can be calibrated on real hardware. In #4164 I ensured that any FSim-compatible gate can be handled on compensation phase. But on characterization step we can support only these 2 gates, because only they are implemented in hardware (so far).

For that:
* Added method `try_convert_syc_or_sqrt_iswap_to_fsim` which is restriction of `try_convert_gate_to_fsim` on gates supported by hardware.
* Used that method in `workflow.py` as gate translator everywhere where `try_convert_sqrt_iswap_to_fsim` was used.
* Fixed a bug in `_merge_into_calibrations` so if there are calibrations for 2 different gates, it wouldn't fail with assertion error, but would append new calibration to the list of calibration.
* Modified a test `test_run_zeta_chi_gamma_calibration_for_moments` so the circuit under test contains gates of 2 types.
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
* Added a method try_convert_gate_to_fsim which can convert to PhaseCalibratedFSimGate wide range of gates, including sqrt(iSWAP), Sycamore and Controlled-Z gates.
* Used this method (instead of try_convert_sqrt_iswap_to_fsim) as default gate translator in all methods in workflow.py.

As a result, compensation step Floquet calibration now works not only for sqrt(iSWAP), but also for:
* all FSim gates (including Sycamore gate);
* PhasedFSim gates (such that zeta=gamma=0); 
* all ISwapPow gates;
* all PhasedISwapPow gates;
* CZPow gates without global phase (including CZ).

This change doesn't affect characterization step the Floquet calibration.
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
…mlib#4248)

Goal of this PR is that circuits having any of these 2-qubit gates (sqrt(iSWAP), Sycamore) can be calibrated on real hardware. In quantumlib#4164 I ensured that any FSim-compatible gate can be handled on compensation phase. But on characterization step we can support only these 2 gates, because only they are implemented in hardware (so far).

For that:
* Added method `try_convert_syc_or_sqrt_iswap_to_fsim` which is restriction of `try_convert_gate_to_fsim` on gates supported by hardware.
* Used that method in `workflow.py` as gate translator everywhere where `try_convert_sqrt_iswap_to_fsim` was used.
* Fixed a bug in `_merge_into_calibrations` so if there are calibrations for 2 different gates, it wouldn't fail with assertion error, but would append new calibration to the list of calibration.
* Modified a test `test_run_zeta_chi_gamma_calibration_for_moments` so the circuit under test contains gates of 2 types.
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.

5 participants