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

Update XEB and Coherent Error Tutorial #5158

Merged
merged 16 commits into from
Jul 16, 2022

Conversation

augustehirth
Copy link
Collaborator

@augustehirth augustehirth commented Mar 29, 2022

Update the XEB and Coherent Error tutorial to:

  • Focus on coherent error and XEB
  • Add comparisons to incoherent error and how XEB handles each
  • Add context, explanations and analysis.

A terminology question: Are they "layers" or "cycles" in the generated XEB circuits?

Out of date description below:

==============================================
Three XEB tutorials:

  • XEB and Coherent Error
  • Parallel XEB
  • Isolated XEB

consist of mostly the same code, and lack key context/explanations in multiple places. The new, replacement tutorial, Coherent vs Incoherent Error with XEB, merges the existing content and adds explanations and some additional analysis.

Edit: I am unsure how to best resolve the codeowners file CI checks.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CirqBot CirqBot added the size: XL lines changed >1000 label Mar 29, 2022
@mpharrigan
Copy link
Collaborator

This is maybe a philosophical difference, but (clearly -- as I wrote the original notebooks) I prefer many, short tutorials that focus on a particular topic rather than trying to show everything at once in as clever a fashion as possible.

In particular, I think the machinery required to have all the different noise models going at once hurts rather than helps; and makes it much less useful as a reference for others to copy out code snippets for their own use.

I'm not impartial though. Curious to hear others' thoughts

@augustehirth
Copy link
Collaborator Author

augustehirth commented Mar 29, 2022

I fundamentally agree with the usefulness of short tutorials, and their use as a (single or set of) code snippets to copy is something I undervalued when writing this. My original motivation was primarily to reduce repetition. Repetition can be very useful to reinforce ideas, but I think 3 tutorials with ~80% functionally identical code needed to be changed somehow. See go/xeb_tutorial_comparison. Additionally, when two similar but different approaches are compared, it seems more useful for me for them to be side-by-side on the same page, instead of in two different pages.

That said, while merging their contents would definitely create a longer tutorial, I also probably got way too excited and went overboard with the additional explanations/analysis, making it much longer than it needs to be. Perhaps a happy medium could be achieved, by shortening things and somehow making it more usable as a code snippet...

This is all my opinion though, so so thank you for your feedback and I'd appreciate any more you or others may have on this.

====================================
Updated: Refocused to just update the XEB and Coherent Error tutorial, left the Parallel XEB and Isolated XEB tutorials unchanged, to serve as simple and straightforward code snippets. I think it would be good to combine Parallel XEB and Isolated XEB into one, because the isolated one is a special case of the parallel one, but that's for another time/PR.

Refine focus to coherent error
Add comparisons to incoherent error
Add context/explanations/analysis
@augustehirth augustehirth changed the title Consolidate three XEB tutorials into one, with additional context Update XEB and Coherent Error Tutorial Apr 7, 2022
@augustehirth
Copy link
Collaborator Author

To note, #5243 performs the previously mentioned merge of Parallel XEB and Isolated XEB, into one straightforward tutorial that hopefully serves well to copy from. In comparison, this submission elaborates only on the existing XEB and Coherent Error tutorial by comparing XEB's effect on coherent vs incoherent noise. A meaningful amount of explanation and redundant code has been pared from this version to refine it's focus on the experiment results.

docs/qcvv/coherent_vs_incoherent_xeb.ipynb Outdated Show resolved Hide resolved
docs/qcvv/coherent_vs_incoherent_xeb.ipynb Outdated Show resolved Hide resolved
docs/qcvv/coherent_vs_incoherent_xeb.ipynb Outdated Show resolved Hide resolved
docs/qcvv/coherent_vs_incoherent_xeb.ipynb Outdated Show resolved Hide resolved
docs/qcvv/coherent_vs_incoherent_xeb.ipynb Outdated Show resolved Hide resolved
"source": [
"The heatmaps reveal that the estimated error is quite similar when compared across qubits in the mock device. \n",
"\n",
"The coherent error heatmap shows the most variation in qubit pair fidelity error, though all values are within `0.003` of each other. The perturbed `SQRT_ISWAP` gate produces a consistent change in angle that interacts differently with the random single-qubit rotations added in the two-qubit circuits of the `circuit_library`, causing this variance.\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not totally clear to me what the point of the diagrams here is. If we are trying to show a higher variance, we could make the phi angles random, rather than uniformly pi/16. This would be more like hardware as well, where different couplers drift at different rates and directions.

Copy link
Collaborator Author

@augustehirth augustehirth Apr 19, 2022

Choose a reason for hiding this comment

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

I tried changing this to create a randomly selected phi angle for each qubit pair in the range [1/16*pi, 1*pi]. The optimization/parameter fitting found some values for phi that were not the same or very close to the original, but presumably are equivalent w.r.t. the PhasedFSimGate. What randomly selected values of phi would be good here for this SQRT_ISWAP/PhasedFSIM gate?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's probably too big of a range. Suggest [0, 2/16*pi]

docs/qcvv/coherent_vs_incoherent_xeb.ipynb Outdated Show resolved Hide resolved
docs/qcvv/coherent_vs_incoherent_xeb.ipynb Outdated Show resolved Hide resolved
docs/qcvv/coherent_vs_incoherent_xeb.ipynb Outdated Show resolved Hide resolved
docs/qcvv/coherent_vs_incoherent_xeb.ipynb Outdated Show resolved Hide resolved
@augustehirth
Copy link
Collaborator Author

Hi @dstrain115 thank you for your review! I've resolved most of your comments and followed up on a few that haven't been completely resolved. I am unsure if you get notified when I add review response comments, so I've tagged you here just in case.

@mpharrigan
Copy link
Collaborator

Just checking in since I first opined long ago.

I like the new direction this PR has taken, namely beefing up the noise model notebook and leaving the "just run it" notebooks alone :)

@mpharrigan
Copy link
Collaborator

@dstrain115 wanna follow up so we can get this merged?

@augustehirth
Copy link
Collaborator Author

Should be updated now.

Associated redirect cl/461225511

Copy link
Collaborator

@dstrain115 dstrain115 left a comment

Choose a reason for hiding this comment

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

The use of multiprocessing in the colab scares me a bit, but other than that, it looks good to me! Definitely an improvement over the previous tutorial!

},
"outputs": [],
"source": [
"import multiprocessing\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need multiprocessing? I am a little worried this may cause problems depending on which environment it is run in.

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 think it would take too long without it. Since it works on Colab, I'll keep it as it is unless something pops up.

@augustehirth augustehirth added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 15, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 15, 2022
@CirqBot CirqBot merged commit 618cab2 into quantumlib:master Jul 16, 2022
@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 Jul 16, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
Update the XEB and Coherent Error tutorial to: 
- Focus on coherent error and XEB
- Add comparisons to incoherent error and how XEB handles each
- Add context, explanations and analysis. 

A terminology question: Are they "layers" or "cycles" in the generated XEB circuits?

Out of date description below:

==============================================
Three XEB tutorials: 
- XEB and Coherent Error
- Parallel XEB
- Isolated XEB

consist of mostly the same code, and lack key context/explanations in multiple places. The new, replacement tutorial, Coherent vs Incoherent Error with XEB, merges the existing content and adds explanations and some additional analysis.

Edit: I am unsure how to best resolve the codeowners file CI checks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: XL lines changed >1000
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants