Create workflow for Z-phase calibration #6728

NoureldinYosri · 2024-09-13T14:52:35Z

This calibration workflow is created for excitation preserving 2-qubit gates and assumes an error model that can be described with small random z-rotations

0: ───Rz(a)───two_qubit_gate───Rz(c)───
                │
1: ───Rz(b)───two_qubit_gate───Rz(d)───

for some small angles a, b, c, and d.

when the error model doesn't apply the workflow may give absured numbers (e.g. fidilities $\notin [0, 1]$). The fidilities can be slightly outside the $[0, 1]$ interval because it's a statistical estimate as can be seen in the following figure which compares the estimated fidelity of a CZ surrounded by random z rotations before and after calibration

test notebook: https://colab.sandbox.google.com/drive/10gZ5dggYKH_xSsCJFpg__GakxvoaZDIi

codecov · 2024-09-19T07:01:48Z

Codecov Report

Attention: Patch coverage is 98.15951% with 3 lines in your changes missing coverage. Please review.

Project coverage is 97.84%. Comparing base (2c914ce) to head (296922d).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
cirq-core/cirq/experiments/z_phase_calibration.py	96.61%	2 Missing ⚠️
cirq-core/cirq/experiments/two_qubit_xeb.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6728      +/-   ##
==========================================
- Coverage   97.85%   97.84%   -0.01%     
==========================================
  Files        1081     1083       +2     
  Lines       93370    93532     +162     
==========================================
+ Hits        91364    91519     +155     
- Misses       2006     2013       +7

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

eliottrosenberg

This is really great, Nour!!

A few nits:
I think the default values in XEBPhasedFSimCharacterizationOptions should be

    characterize_theta=False,
    characterize_phi=False,
    zeta_default=0,
    chi_default=0,
    gamma_default=0

As written, the parameters like zeta_default are optional, but if you leave them as None, then it complains, e.g. ValueError: zeta_default was not set..

When running this, I get a lot of runtime warnings invalid value encountered in equal coming from cirq/value/value_equality_attr.py:82.

I tried running this twice on hardware. The first time, I got zeta=0.014562062933646333, chi=0.0651809408437691, gamma=-0.0799040390729371 and the second time, I got zeta=0.037173, chi=0.048721, gamma=-0.076402, which is basically consistent, but it would be good to quantify statistical uncertainty if that isn't too difficult.

A method for making a plot like the one that you posted would be really helpful: one that shows the fidelity vs depth for the original and reoptimized gates.

In order to use the results of this characterization tool, a simple transformer that replaces the original gates with the appropriate reoptimized gates could be helpful, but that can go in a separate PR.

pavoljuhas

Please reduce the test duration to the order of seconds and make the axes argument in plot optional. Otherwise LGTM.

cirq-core/cirq/experiments/z_phase_calibration.py

cirq-core/cirq/experiments/xeb_fitting.py

pavoljuhas · 2024-10-07T21:55:07Z

cirq-core/cirq/experiments/z_phase_calibration.py

+
+def plot_z_phase_calibration_result(
+    before_after_df: 'pd.DataFrame',
+    axes: np.ndarray[Sequence[Sequence['plt.Axes']], np.dtype[np.object_]],


Most other plotting functions have axes as an optional argument, which is created and returned if not provided.
Can we follow the same pattern here? Also please simplify the axes type to Sequence["plot.Axes"].

Also please simplify the axes type to Sequence["plot.Axes"]

when I do that I get

cirq-core/cirq/experiments/z_phase_calibration.py:250: error: Incompatible types in assignment (expression has type "ndarray[Any, dtype[Any]]", variable has type "Sequence[Axes] | None") [assignment] cirq-core/cirq/experiments/z_phase_calibration.py:251: error: Item "Sequence[Axes]" of "Sequence[Axes] | Any" has no attribute "flatten" [union-attr] cirq-core/cirq/experiments/z_phase_calibration.py:270: error: No return value expected [return-value]

pavoljuhas · 2024-10-07T21:56:19Z

cirq-core/cirq/experiments/z_phase_calibration.py

+def plot_z_phase_calibration_result(
+    before_after_df: 'pd.DataFrame',
+    axes: np.ndarray[Sequence[Sequence['plt.Axes']], np.dtype[np.object_]],
+    *,


@eliottrosenberg - Would it be useful to provide an optional pairs argument if only specific qubit pairs are desired for plotting?

I added that option

pavoljuhas · 2024-10-07T22:11:52Z

cirq-core/cirq/experiments/z_phase_calibration_test.py

+
+
+@pytest.mark.parametrize(['angles', 'error'], _create_tests(n=10, seed=32432432))
+def test_calibrate_z_phases(angles, error):


The tests are very slow and take about 1 minute to run on my desktop with --numprocesses=2 (number of cores on GHA). They make up the bulk of the 20 slowest durations in this PRs CI at https://github.com/quantumlib/Cirq/actions/runs/11152830363/job/30999210792?pr=6728#step:7:507.

Please reduce the number and computational size of the tests so we don't add more than several seconds to the pytest time. The main purpose of unit tests is to exercise all code branches and check a few small and quick computations.
If you need to run larger tests, please move them to a separate test method decorated with @pytest.mark.slow.

they should be faster now, ptal

pavoljuhas · 2024-10-07T22:30:34Z

cirq-core/cirq/experiments/z_phase_calibration_test.py

+    _, axes = plt.subplots(1, 2)
+    plot_z_phase_calibration_result(before_after_df=df, axes=axes)
+
+    np.testing.assert_allclose(axes[0].lines[0].get_xdata().astype(float), [1, 2, 3])


It seems the arrays passed to plotting have dtype=object, otherwise this should work without astype(float).
Is it feasible to convert to numerical arrays in or before the plotting code?

the arrays are lists of floats/ints ... eveny when I explicitly passed np.array(..., dtype=float) I still got the same issue

cirq-core/cirq/experiments/two_qubit_xeb.py

eliottrosenberg · 2024-10-30T20:20:29Z

cirq-core/cirq/experiments/__init__.py

@@ -82,3 +82,6 @@
    parallel_two_qubit_xeb as parallel_two_qubit_xeb,
    run_rb_and_xeb as run_rb_and_xeb,
 )
+
+
+from cirq.experiments.z_phase_calibration import z_phase_calibration_workflow, calibrate_z_phases


It's a little redundant to have both calibrate_z_phases and z_phase_calibration_workflow. Can we just have one thing with the functionality of z_phase_calibration_workflow?

are you sure?

On an unrelated note, please use the from somewhere import foo as foo so that the new names can be imported from cirq.experiments without raising mypy error. (Ref: #6717)

cirq-core/cirq/experiments/z_phase_calibration.py

cirq-core/cirq/experiments/z_phase_calibration_test.py

NoureldinYosri · 2024-10-31T01:52:10Z

@pavoljuhas ptal

@eliottrosenberg I was not able to reproduce the warnings you recieved locally

pavoljuhas

Some minor suggestions, otherwise LGTM.

pavoljuhas · 2024-11-04T20:10:42Z

cirq-core/cirq/experiments/__init__.py

@@ -82,3 +82,6 @@
    parallel_two_qubit_xeb as parallel_two_qubit_xeb,
    run_rb_and_xeb as run_rb_and_xeb,
 )
+
+
+from cirq.experiments.z_phase_calibration import z_phase_calibration_workflow, calibrate_z_phases


On an unrelated note, please use the from somewhere import foo as foo so that the new names can be imported from cirq.experiments without raising mypy error. (Ref: #6717)

pavoljuhas · 2024-11-04T22:07:59Z

cirq-core/cirq/experiments/z_phase_calibration.py

+    cycle_depths: Sequence[int] = tuple(np.arange(3, 100, 20)),
+    random_state: 'cirq.RANDOM_STATE_OR_SEED_LIKE' = None,
+    atol: float = 1e-3,
+    pool_or_num_workers: Optional[Union[int, 'multiprocessing.pool.Pool']] = None,


Please make a single-process execution the default behavior of the pool argument.
Running via multiprocessing has previously caused unexpected problems in #6674.
It is better if users explicitly ask for multiprocessing so there is a better chance that
potential bugs appear then rather as a default behavior.

I would also recommend to split the pool_or_num_workers argument into two exclusive arguments, pool and num_workers: Optional[int].
The invocation via num_workers could then use a recursive call as

with multiprocessing.Pool(num_workers) as pool: return z_phase_calibration_workflow(sampler, ..., pool=pool)

which would be more robust for closing the pool in case of exception, you'd also won't need to track a local_pool flag.

Please make a single-process execution the default behavior of the pool argument

this was the initial behaviour but @eliottrosenberg requested the default be multiprocessing

I would also recommend to split the pool_or_num_workers argument into two exclusive arguments, pool and num_workers: Optional[int]

I condidered this but I thought the api would be a bit convoluted

@pavoljuhas I think that most users would use it with the default values without passing a parallel pool, and it is very slow that way.

My concern is all other functions with pool-like argument interpret pool=None as run in series.
If reading z_phase_calibration_workflow(..., pool_or_num_workers=None) my expectation would be to have no pool and no workers, but in fact this starts a pool of ncpu size, which may cause memory problems on hosts with many CPUs.

Can we instead use some reasonable default, say 4, and not accept None, ie, make that argument of Union[int, 'multiprocessing.pool.Pool'] type?

Also renaming to num_workers_or_pool might give a better hint to pass integer and make distinction from pool-like arguments.

@NoureldinYosri

cirq-core/cirq/experiments/z_phase_calibration.py

pavoljuhas · 2024-11-04T22:12:02Z

cirq-core/cirq/experiments/z_phase_calibration.py

+    cycle_depths: Sequence[int] = tuple(np.arange(3, 100, 20)),
+    random_state: 'cirq.RANDOM_STATE_OR_SEED_LIKE' = None,
+    atol: float = 1e-3,
+    pool_or_num_workers: Optional[Union[int, 'multiprocessing.pool.Pool']] = None,


Please make single process the default and split to 2 arguments as suggested above for z_phase_calibration_workflow.

@eliottrosenberg

cirq-core/cirq/experiments/z_phase_calibration.py

pavoljuhas

LGTM with a few spelling suggestions. Thank you!

Co-authored-by: Pavol Juhas <[email protected]>

comments addressed

NoureldinYosri requested review from mrwojtek, vtomole and a team as code owners September 13, 2024 14:52

NoureldinYosri requested review from MichaelBroughton and eliottrosenberg September 13, 2024 14:52

NoureldinYosri added 8 commits September 18, 2024 05:46

checkpoint

ade51ee

checkpoint

145880e

Create workflow for Z-phase calibration

9a9d06f

update deps

d37ea60

increase error

717155c

nit

b053a93

nit

eadda56

Add test

07ba30a

NoureldinYosri force-pushed the z_cal2 branch from fa125bb to 07ba30a Compare September 18, 2024 12:47

NoureldinYosri mentioned this pull request Sep 18, 2024

cirq rigetti cause CI failure for pytest and mypy at head #6732

Closed

NoureldinYosri and others added 3 commits September 19, 2024 07:11

Merge branch 'main' into z_cal2

8c244e7

checkpoint

552b7df

coverage

6f71abf

eliottrosenberg requested changes Sep 23, 2024

View reviewed changes

Add plotting method and calculate the variance of estimated fidelity

39b1537

NoureldinYosri force-pushed the z_cal2 branch from 87cb95a to 39b1537 Compare October 2, 2024 20:29

NoureldinYosri and others added 3 commits October 2, 2024 21:29

Merge branch 'main' into z_cal2

e2cac7d

docstring

9f42000

coverage

26593a1

NoureldinYosri requested a review from eliottrosenberg October 2, 2024 21:46

change qubit names

39e0a6d

pavoljuhas requested changes Oct 7, 2024

View reviewed changes

CirqBot added the size: L 250< lines changed <1000 label Oct 12, 2024

Merge branch 'main' into z_cal2

d0a9583

eliottrosenberg previously requested changes Oct 30, 2024

View reviewed changes

NoureldinYosri and others added 3 commits October 30, 2024 16:13

checkpoint

530dc2d

address comments

55c47a5

Merge branch 'main' into z_cal2

0cb890d

NoureldinYosri requested review from pavoljuhas and eliottrosenberg and removed request for MichaelBroughton October 31, 2024 01:52

pavoljuhas requested changes Nov 4, 2024

View reviewed changes

pavoljuhas reviewed Nov 4, 2024

View reviewed changes

cirq-core/cirq/experiments/z_phase_calibration.py Show resolved Hide resolved

address comments

5d668e2

NoureldinYosri requested a review from pavoljuhas November 7, 2024 20:56

nit

a372c8e

NoureldinYosri force-pushed the z_cal2 branch from b241ec2 to a372c8e Compare November 7, 2024 21:44

Merge branch 'main' into z_cal2

fcb2d2a

pavoljuhas reviewed Nov 7, 2024

View reviewed changes

cirq-core/cirq/experiments/z_phase_calibration.py Outdated Show resolved Hide resolved

pavoljuhas reviewed Nov 7, 2024

View reviewed changes

cirq-core/cirq/experiments/z_phase_calibration.py Outdated Show resolved Hide resolved

pavoljuhas reviewed Nov 7, 2024

View reviewed changes

cirq-core/cirq/experiments/z_phase_calibration.py Outdated Show resolved Hide resolved

pavoljuhas reviewed Nov 7, 2024

View reviewed changes

cirq-core/cirq/experiments/z_phase_calibration.py Outdated Show resolved Hide resolved

pavoljuhas approved these changes Nov 7, 2024

View reviewed changes

NoureldinYosri and others added 4 commits November 7, 2024 15:55

Update cirq-core/cirq/experiments/z_phase_calibration.py

d9a8282

Co-authored-by: Pavol Juhas <[email protected]>

Update cirq-core/cirq/experiments/z_phase_calibration.py

b19cee9

Co-authored-by: Pavol Juhas <[email protected]>

Update cirq-core/cirq/experiments/z_phase_calibration.py

360aee1

Co-authored-by: Pavol Juhas <[email protected]>

Update cirq-core/cirq/experiments/z_phase_calibration.py

296922d

Co-authored-by: Pavol Juhas <[email protected]>

NoureldinYosri enabled auto-merge (squash) November 7, 2024 23:56

NoureldinYosri merged commit 59be462 into quantumlib:main Nov 8, 2024
37 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create workflow for Z-phase calibration #6728

Create workflow for Z-phase calibration #6728

NoureldinYosri commented Sep 13, 2024 •

edited

Loading

codecov bot commented Sep 19, 2024 •

edited

Loading

eliottrosenberg left a comment

pavoljuhas left a comment

pavoljuhas Oct 7, 2024

NoureldinYosri Oct 31, 2024

pavoljuhas Oct 7, 2024

NoureldinYosri Oct 31, 2024

pavoljuhas Oct 7, 2024

NoureldinYosri Oct 31, 2024

pavoljuhas Oct 7, 2024

NoureldinYosri Oct 31, 2024

eliottrosenberg Oct 30, 2024

NoureldinYosri Oct 31, 2024

pavoljuhas Nov 4, 2024

NoureldinYosri commented Oct 31, 2024

pavoljuhas left a comment

pavoljuhas Nov 4, 2024

pavoljuhas Nov 4, 2024

NoureldinYosri Nov 4, 2024 •

edited

Loading

eliottrosenberg Nov 5, 2024

pavoljuhas Nov 7, 2024 •

edited

Loading

NoureldinYosri Nov 7, 2024

pavoljuhas Nov 4, 2024

NoureldinYosri Nov 4, 2024

pavoljuhas left a comment



		@pytest.mark.parametrize(['angles', 'error'], _create_tests(n=10, seed=32432432))
		def test_calibrate_z_phases(angles, error):

Create workflow for Z-phase calibration #6728

Create workflow for Z-phase calibration #6728

Conversation

NoureldinYosri commented Sep 13, 2024 • edited Loading

codecov bot commented Sep 19, 2024 • edited Loading

Codecov Report

eliottrosenberg left a comment

Choose a reason for hiding this comment

pavoljuhas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NoureldinYosri commented Oct 31, 2024

pavoljuhas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NoureldinYosri Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pavoljuhas Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pavoljuhas left a comment

Choose a reason for hiding this comment

NoureldinYosri commented Sep 13, 2024 •

edited

Loading

codecov bot commented Sep 19, 2024 •

edited

Loading

NoureldinYosri Nov 4, 2024 •

edited

Loading

pavoljuhas Nov 7, 2024 •

edited

Loading