Update "Compare to Calibration" tutorial into "Identifying Hardware Changes" with Simultaneous Readout and additional context #4552
Conversation
augustehirth
requested review from
cduck,
vtomole,
wcourtney and
a team
as code owners
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
Thanks so much for opening the PR. This is a good first pass on an update for this tutorial, there are a few outstanding changes that need to be made before we can get closer to merging.
Also, if we are going to change the name of the tutorial, we will need to update https://github.com/quantumlib/Cirq/blob/master/docs/_book.yaml as well to update the name.
A lot of the reasoning behind some of the suggestions for changes can be found here: https://developers.google.com/style.
| "source": [ | ||
| "# Qubit Error Metrics\n", | ||
| "\n", | ||
| "This tutorial will focus on the following two metrics for our examples:\n", |
There was a problem hiding this comment.
We do have a tutorial that lists all of the available metrics that our system can provide to a user (https://quantumai.google/cirq/tutorials/google/visualizing_calibration_metrics). Do you think it would be a good idea to link out to that tutorial and emphasize something to the effect of: "Using just these two metrics from this big list here <link> you can characterize drift" ? instead of having to spend so much time re-introducing the metrics below.
There was a problem hiding this comment.
I've significantly shortened and simplified this section, but I still think it is valuable for there to be a short explanation of the metrics we're using in the tutorial, so that the tutorial is more or less one cohesively digestible meal of information. Going back and forth to other pages to understand core concepts seems like it would interrupt the overall understanding process, at least for me. I've cut the explanations of the other metrics.
|
Thank you for all the detailed feedback. I'm working through the changes now. |
| "outputId": "81b2dc67-7347-471c-c743-de870803da61" | ||
| }, | ||
| "source": [ | ||
| "sq_result = cirq.estimate_parallel_single_qubit_readout_errors(sampl |
There was a problem hiding this comment.
This analysis is quite confusing and hand-wavy. Is this really our best practice? Our advice is to stare at these heatmaps and try to manually glean some information out of them? We are not going to tell them to refit bad two-qubit gates or to use any readout mitigation techniques?
There was a problem hiding this comment.
The next steps section at the bottom provide links for using calibration for compensation and for other resources in the tutorial. This tutorial at the moment focuses on identifying drift and picking qubits, not how you should change your circuit to fit the qubits selected. That said, I agree with you that we need better defined best practices for how to build circuits given this data, and this tutorial may be the correct place for that. I'd greatly appreciate any information or sources to read up on information that should possibly be in this tutorial.
| "outputId": "81b2dc67-7347-471c-c743-de870803da61" | ||
| }, | ||
| "source": [ | ||
| "sq_result = cirq.estimate_parallel_single_qubit_readout_errors(sampl |
There was a problem hiding this comment.
I think each of these tutorials actually use client-side calibration techniques and not the Calibration API.
There was a problem hiding this comment.
This is an especially important point that I don't know how to immediately resolve. I attempted to use the shortest available way to collect the data, and focus the tutorial on comparison. This means using cirq.experiments.xeb_sampling.sample_2q_xeb_circuits and cirq.estimate_parallel_single_qubit_readout_errors, which, as far as I can tell, don't use the Calibration API under the hood. The alternative would be to set up and run a cirq.google.CalibrationLayer manually, or sending a large dummy circuit into prepare_characterization_for_operations. This is an option but doesn't seem to be presented the other tutorials. For now I'll fix the references to the Calibration API.
- Removed usage of "drift" outside of "thermal drift" - Separated out "Hardware Changes" section to describe the types and causes, as well as replacing entire section with suggestion - Some minor wording refinement - Renamed to "identifying hardware changes" in line with "drift" change
|
Version 3 has significant changes based on feedback, including removal of incorrect use of "drift", as well as addition/rework of "hardware changes" section. |
| "device = qcs_objects.device\n", | ||
| "sampler = qcs_objects.sampler\n", | ||
| "\n", | ||
| "# Get qubit set\n", | ||
| "qubits = device.qubit_set()\n", | ||
| "\n", | ||
| "# Limit device qubits to only those before row/column `device_limit`\n", | ||
| "device_limit = 10 #@param {type:\"integer\"}\n", | ||
| "qubits = {qb for qb in qubits if qb.row<device_limit and qb.col<device_limit}\n", | ||
| "\n", | ||
| "# Visualize the device qubits\n", | ||
| "print(cg.devices.XmonDevice(0,0,0,qubits))" |
There was a problem hiding this comment.
It might be a little more relevant if we used a Sycamore device here, since those are the ones that are most prevalent in the QCS fleet.
There was a problem hiding this comment.
The intent is that the user puts in a QCS project_id and actual-device processor_id, and the examples reflect this. If you mean the cg.devices.XmonDevice at the bottom, it's just to show the device graph with connectivity, especially if you limit the qubit set. I don't think there's another way to do this currently, especially because print(device) here wouldn't show qubit connectivity.
There was a problem hiding this comment.
cg.devices.XmonDevice at the bottom, it's just to show the device graph with connectivity, especially if you limit the qubit set.
Right, but we don't host any xmon devices on QCS anymore. cg.Sycamore23 is probably a more relevant device since those are what we host. I think just changing the line to cg.Sycamore23 over cg.devices.XmonDevice(...) is all we need.
| "cycle_depths = np.arange(3, 100, 20)" | ||
| ], | ||
| "execution_count": null, | ||
| "outputs": [] |
There was a problem hiding this comment.
Is there any intermediate output we could show here ?
There was a problem hiding this comment.
I haven't found anything suitable yet. The closest thing I've thought of is to print combinations_by_layer or one of it's elements, but I don't think it would provide much useful insight into the tutorial's problem. That data structure is a little hard to read easily, and I think one of the XEB tutorials would be a better place to go into how cirq.experiments.random_quantum_circuit_generation.get_random_combinations_for_device works.
| "pxeb_results = {\n", | ||
| " pair: (1.0 - fidelity) / (4 / 3) #Scalar to get Pauli error\n", | ||
| " for (_, _, pair), fidelity in fidelities.layer_fid.items()\n", | ||
| "}" | ||
| ], | ||
| "execution_count": null, |
There was a problem hiding this comment.
Stylistic question: Is it worth showing all the visualizations at the end or do you think it's worth showing the baseline comparison for XEB here along with the baseline readout comparison in the readout section and then maybe the more advanced comparison (the deltas) at the end ?
There was a problem hiding this comment.
This was how it was originally. I elected to put all three (calibration, current, deltas) graphs together shortly before the sections discussing them for easy reference, and to show the user the big picture all at once. Having the same graphs (without deltas) duplicated and printed earlier, without the explanations provided later, seems like it would unnecessarily delay getting to the important part of the tutorial (the comparisons and explanations). I'm definitely open to them being potentially useful, but I'm not sure how right off the bat.
There was a problem hiding this comment.
Though along this line of reasoning the original calibration data's heatmaps should be removed, so I'm unsure what's best here.
Removed authorization link and code Removed try-except statement Removed used-once function Removed assert statement Moved import to top of cell Added note on project/processor_id(s)
|
This looks good to me. I think after the notebook formatting fixes, changing over the entry in |
augustehirth
changed the title
MichaelBroughton
added
cla: yes
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
google-cla
bot
added
cla: no
and removed
cla: yes
|
Any updates on the CLA status here @augustehirth ? |
|
CLA status is moving forwards, but very slowly. Thanks for your patience. |
|
@googlebot I signed it! |
google-cla
bot
added
cla: yes
|
@MichaelBroughton CLA should be resolved, is there anything else that needs to be changed here? |
MichaelBroughton
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
…hanges" with Simultaneous Readout and additional context (quantumlib#4552) A large rework of the "Compare to Calibration" tutorial to emphasize the use of Parallel XEB and Simultaneous Readout (new) to identify if and how qubits have drifted since the previous maintenance calibration. Adds contextual information on what drift is, how it occurs, and what you can do about it once spotted. TODO/Things I don't have answers for yet: - Relative linking for API reference pages - Need more links/references on qubit picking/mapping/routing. - An email address should be supplied for the quantum engine support team at the end of #Heatmap Observations - Edit: Renaming the file without losing the previous contributors' commit history. More importantly, looking for any corrections, feedback or advice on improving this. Thanks!
…hanges" with Simultaneous Readout and additional context (quantumlib#4552) A large rework of the "Compare to Calibration" tutorial to emphasize the use of Parallel XEB and Simultaneous Readout (new) to identify if and how qubits have drifted since the previous maintenance calibration. Adds contextual information on what drift is, how it occurs, and what you can do about it once spotted. TODO/Things I don't have answers for yet: - Relative linking for API reference pages - Need more links/references on qubit picking/mapping/routing. - An email address should be supplied for the quantum engine support team at the end of #Heatmap Observations - Edit: Renaming the file without losing the previous contributors' commit history. More importantly, looking for any corrections, feedback or advice on improving this. Thanks!
A large rework of the "Compare to Calibration" tutorial to emphasize the use of Parallel XEB and Simultaneous Readout (new) to identify if and how qubits have drifted since the previous maintenance calibration. Adds contextual information on what drift is, how it occurs, and what you can do about it once spotted.
TODO/Things I don't have answers for yet:
More importantly, looking for any corrections, feedback or advice on improving this. Thanks!