Skip to content

Conversation

@mpharrigan
Copy link
Collaborator

@mpharrigan mpharrigan commented Jan 12, 2021

  1. Add an explicit function for generating two-qubit fragments. These are important building blocks for doing "parallel XEB" e.g. for characterization/calibration. The new signature accepts any type of Qid and doesn't require a pattern argument.
  2. I changed some of the internals of the random circuit generation to use the relevant Cirq type, like Moment instead of Dict[qubit, op].
  3. Refactored the docstrings to associate specific elements of the circuit generation to the argument docs that control it. I personally think this is more organized than the wall-o-paragraph approach where every corner case of the function is documented up top
  4. Add a notebook that expands upon the mathy docstring of cirq.experiments.fidelity_estimation.least_squares_xeb_fidelity_from_expectations with nice diagrams and plots. Let me know if this should be in docs/ or docs/xeb/ or docs/characterization/.

@XiaoMiQC, who I can't add as reviewer

 - Two-qubit generation (no "pattern" argument, can do line qubits
 - use cirq objects in random circuit construction
 - Notebook showing the theory for the fidelity function.
@mpharrigan mpharrigan requested review from a team, cduck and vtomole as code owners January 12, 2021 23:10
@google-cla google-cla bot added the cla: yes Makes googlebot stop complaining. label Jan 12, 2021
@mpharrigan
Copy link
Collaborator Author

@balopat I see there's a check that means I can't use cirq.contrib in notebooks; specifically cirq.contrib.svg.SVGCircuit. I think this is a really important utility to make rich notebooks with pretty circuit diagrams. Should we revisit this policy? Move SVG out of contrib? I don't necessarily want this to block the current PR

@mpharrigan
Copy link
Collaborator Author

I'm also somewhat stumped by the notebook check error, per usual

@balopat
Copy link
Contributor

balopat commented Jan 23, 2021

I'm also somewhat stumped by the notebook check error, per usual

I'm sorry you have to deal with this :/ #3603

@mpharrigan
Copy link
Collaborator Author

I can't reproduce the error when running pytest -m slow dev_tools/notebook_test.py -v locally

@mpharrigan
Copy link
Collaborator Author

However, when running locally I run into the issue that this notebook tries to use a function that was introduced in this PR, but it installs the stable version of cirq from pip, which obviously doesn't work. Do I have to split this into two PRs and then use cirq-nightly in the notebook? I feel like this issue will come up a lot if we want people to document new features with notebooks as they contribute them

@balopat
Copy link
Contributor

balopat commented Jan 24, 2021

However, when running locally I run into the issue that this notebook tries to use a function that was introduced in this PR, but it installs the stable version of cirq from pip, which obviously doesn't work. Do I have to split this into two PRs and then use cirq-nightly in the notebook? I feel like this issue will come up a lot if we want people to document new features with notebooks as they contribute them

This is a general issue with notebooks: they are targeted at the latest stable version. We don't have a concept of "nightly notebooks" yet, but this could be one of the first ones. We also don't test notebook compatibility with master either. We can start to think about this.

For now, as this is a new notebook, and it is not part of the devsite documentation yet, I would just exclude it explicitly from the notebook_tests (the ignore pattern at the top of the file test file), make sure that the notebook actually does install cirq --pre and put in a warning that this notebook is based on unreleased features.

@mpharrigan
Copy link
Collaborator Author

done.

Really, the notebooks should be versioned like Cirq proper. Ideally we should have the notebook tests run against cirq-as-in-the-pr (auto-installed in environment) and publish a colab environment that has cirq pre-installed. I took a look at the tensorflow examples and they don't install tensorflow every time for each notebook

@mpharrigan
Copy link
Collaborator Author

PTAL code owners

Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

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

Approved with nits - I'm going to review the notebook too

@mpharrigan
Copy link
Collaborator Author

@balopat back at you

@balopat balopat added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jan 27, 2021
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jan 27, 2021
@CirqBot CirqBot merged commit 2f9864b into quantumlib:master Jan 27, 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 Jan 27, 2021
@mpharrigan mpharrigan deleted the 2021-01-xeb-analysis branch January 27, 2021 22:54
CirqBot pushed a commit that referenced this pull request Feb 3, 2021
Following #3669.

This wraps up the sampling and simulation functions teased in the previous notebook. It includes utilities for batching sampler requests and multiprocessing simulation. The notebook walks through how coherent error can cause fidelity to decay. In the next PR, we'll add an optimization loop to characterize the mystery angle.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/xeb cla: yes Makes googlebot stop complaining.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants