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

Support multi-qubit measurement in plot_state_histogram #3054

Merged
merged 12 commits into from
Jun 9, 2020

Conversation

AnimeshSinha1309
Copy link
Contributor

@AnimeshSinha1309 AnimeshSinha1309 commented Jun 1, 2020

plot_state_histogram works with single qubit measurements, multi-qubit measurements are now also supported.

Fixes #3031.

plot_state_histogram works with single qubit measurements, multi-qubit measurements are detected by string matching and a warning is raised.
@googlebot
Copy link

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 @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes Makes googlebot stop complaining. and removed cla: no labels Jun 1, 2020
Order of imports and spaces on a line, linter issues fixed.
@AnimeshSinha1309
Copy link
Contributor Author

Please do not review this for now, I want to add the multi-qubit plotting logic here instead of raising a warning, just waiting to know if that is the expected behavior.

H-stacking the different measurements to allow plot_state_histogram to support multi-qubit measurements intermixed with single-qubit.
Removed import of warnings.
@AnimeshSinha1309
Copy link
Contributor Author

I guess now this should be review ready, I removed the warning and added h-stacking the results. Please review.

dabacon
dabacon previously requested changes Jun 1, 2020
Copy link
Collaborator

@dabacon dabacon left a comment

Choose a reason for hiding this comment

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

Could you add a test for this new case?

Copy link
Collaborator

@cduck cduck left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this. It looks good but needs a few unit tests. See my comments and add the tests to visualize_test.py.

cirq/study/visualize.py Outdated Show resolved Hide resolved
cirq/study/visualize.py Show resolved Hide resolved
Removed a frivolous assertion and added tests for the plotting.
Copy link
Collaborator

@cduck cduck left a comment

Choose a reason for hiding this comment

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

Almost done. Can you change the circuits in the tests to verify the endianness is preserved? Something like only do and X on the 2nd and 3rd qubits.

test_plot_state_histogram_multi_2 is essentially the same as test_plot_state_histogram and probably not needed.

cirq/study/visualize_test.py Outdated Show resolved Hide resolved
cirq/study/visualize_test.py Outdated Show resolved Hide resolved
cirq/study/visualize_test.py Show resolved Hide resolved
cirq/study/visualize_test.py Show resolved Hide resolved
cirq/study/visualize_test.py Outdated Show resolved Hide resolved
Suppressed plots to PDFs during testing, updated some copied comments.
@AnimeshSinha1309 AnimeshSinha1309 requested a review from dabacon June 5, 2020 15:19
Applying X on all but one of the qubits, also deleting a test.
@AnimeshSinha1309 AnimeshSinha1309 requested a review from cduck June 8, 2020 05:22
@cduck cduck changed the title Warning for multi-qubit measurement in plot_state_histogram Support multi-qubit measurement in plot_state_histogram Jun 8, 2020
Copy link
Collaborator

@cduck cduck left a comment

Choose a reason for hiding this comment

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

LGTM. Just correct the comment and then I can merge.

c = cirq.Circuit(
cirq.X.on_each(*qubits[1:]),
cirq.measure(*qubits[:2]), # One multi-qubit measurement
cirq.measure_each(*qubits[2:]), # One single-qubit measurement
Copy link
Collaborator

@cduck cduck Jun 8, 2020

Choose a reason for hiding this comment

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

The comment is wrong. It is now two single-qubit measurements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@cduck cduck added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jun 8, 2020
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jun 8, 2020
@CirqBot
Copy link
Collaborator

CirqBot commented Jun 8, 2020

Automerge cancelled: A status check is failing.

@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 8, 2020
Replaced single qubit measurement with multiple qubit.
@AnimeshSinha1309 AnimeshSinha1309 force-pushed the issues/hist-multi branch 3 times, most recently from 8cadd59 to fe415aa Compare June 9, 2020 12:36
@AnimeshSinha1309
Copy link
Contributor Author

@cduck I have no idea what is wrong, the docs fail on some completely separate module:

Warning, treated as error:
/home/runner/work/Cirq/Cirq/docs/generated/cirq.experiments.cross_entropy_benchmarking.rst:6:Error in "autofunction" directive:
1 argument(s) required, 0 supplied.

The commits that were working before are now failing, I just tried to roll back the bunch of commits I made trying to get rid of the error. What do I do about this?

@cduck
Copy link
Collaborator

cduck commented Jun 9, 2020

This error appears unrelated to your code. It could be caused by a newly published version of one of the documentation packages that broke something. @dstrain115, @dabacon Any ideas?

@dabacon
Copy link
Collaborator

dabacon commented Jun 9, 2020

This error appears unrelated to your code. It could be caused by a newly published version of one of the documentation packages that broke something. @dstrain115, @dabacon Any ideas?

I have a fix for this in #3082

@dabacon dabacon added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jun 9, 2020
@CirqBot
Copy link
Collaborator

CirqBot commented Jun 9, 2020

Automerge cancelled: A status check is failing.

@CirqBot CirqBot removed the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jun 9, 2020
@dabacon dabacon merged commit f55090a into quantumlib:master Jun 9, 2020
MichaelBroughton added a commit that referenced this pull request Jun 12, 2020
* [SVG] Noise hack and font fix (#3076)


 - Specify the font. This should not be a change for viewing in Jupyter notebook but fixes opening the svg data in e.g. inkscape where the default font is different and the boxes aren't the right size
 - Don't display virtual tags (for noise) xref #2905

* Support multi-qubit measurement in plot_state_histogram (#3054)

* Add generated code for batch.proto (#3086)

Review: @dstrain115

* Fix flakes in random_circuit_test (#3084)

* Added api docs gen (copy from TFQ).

* updated site_path prefix.

* removed pre-built.

* rename.

* move to dev_tools.

* removed __future__ and formatting.

* yet more formatting.

Co-authored-by: Matthew Harrigan <[email protected]>
Co-authored-by: Animesh Sinha <[email protected]>
Co-authored-by: Matthew Neeley <[email protected]>
Co-authored-by: Dave Bacon <[email protected]>
MichaelBroughton added a commit that referenced this pull request Jun 12, 2020
* [SVG] Noise hack and font fix (#3076)


 - Specify the font. This should not be a change for viewing in Jupyter notebook but fixes opening the svg data in e.g. inkscape where the default font is different and the boxes aren't the right size
 - Don't display virtual tags (for noise) xref #2905

* Support multi-qubit measurement in plot_state_histogram (#3054)

* Add generated code for batch.proto (#3086)

Review: @dstrain115

* Fix flakes in random_circuit_test (#3084)

* Fix Quirk import extra gates bug

* Move image files to docs/images/

* Move unneeded site assets

* Move Sphinx templates, Makefile, conf, and init

* Added api docs gen (copy from TFQ). (#3089)

* [SVG] Noise hack and font fix (#3076)


 - Specify the font. This should not be a change for viewing in Jupyter notebook but fixes opening the svg data in e.g. inkscape where the default font is different and the boxes aren't the right size
 - Don't display virtual tags (for noise) xref #2905

* Support multi-qubit measurement in plot_state_histogram (#3054)

* Add generated code for batch.proto (#3086)

Review: @dstrain115

* Fix flakes in random_circuit_test (#3084)

* Added api docs gen (copy from TFQ).

* updated site_path prefix.

* removed pre-built.

* rename.

* move to dev_tools.

* removed __future__ and formatting.

* yet more formatting.

Co-authored-by: Matthew Harrigan <[email protected]>
Co-authored-by: Animesh Sinha <[email protected]>
Co-authored-by: Matthew Neeley <[email protected]>
Co-authored-by: Dave Bacon <[email protected]>

* Move doc tools to /dev_tools/docs/

* Move examples index into tutorials/

* Move basics tutorial into tutorials/

* Encode URL bracket so not mistaken for template.

* Update references to moves docs and image files.

* Update references to docs_coverage_test.py

* Format py with check/format-incremental --apply

* Move Sphinx RST files

* Add _index.yaml for site

* Add initial _book.yaml for leftnav

* Manually format docs/_sphinx/conf.py for CI

* Update references to moved api.rst file

* Move docs/_sphinx to dev_tools/docs/sphinx

* Update README.rst path for snippets_test.py

* Move run_doctest to dev_tools/docs/

* Move build-docs.sh to dev_tools/docs/sphinx/

* Update references to build-docs.sh

* Remove build_docs CI test

Co-authored-by: Matthew Harrigan <[email protected]>
Co-authored-by: Animesh Sinha <[email protected]>
Co-authored-by: Matthew Neeley <[email protected]>
Co-authored-by: Dave Bacon <[email protected]>
Co-authored-by: Matteo Pompili <[email protected]>
Co-authored-by: MichaelBroughton <[email protected]>
balopat pushed a commit to balopat/Cirq that referenced this pull request Aug 18, 2020
* [SVG] Noise hack and font fix (quantumlib#3076)

 - Specify the font. This should not be a change for viewing in Jupyter notebook but fixes opening the svg data in e.g. inkscape where the default font is different and the boxes aren't the right size
 - Don't display virtual tags (for noise) xref quantumlib#2905

* Support multi-qubit measurement in plot_state_histogram (quantumlib#3054)

* Add generated code for batch.proto (quantumlib#3086)

Review: @dstrain115

* Fix flakes in random_circuit_test (quantumlib#3084)

* Added api docs gen (copy from TFQ).

* updated site_path prefix.

* removed pre-built.

* rename.

* move to dev_tools.

* removed __future__ and formatting.

* yet more formatting.

Co-authored-by: Matthew Harrigan <[email protected]>
Co-authored-by: Animesh Sinha <[email protected]>
Co-authored-by: Matthew Neeley <[email protected]>
Co-authored-by: Dave Bacon <[email protected]>
balopat pushed a commit to balopat/Cirq that referenced this pull request Aug 18, 2020
* [SVG] Noise hack and font fix (quantumlib#3076)

 - Specify the font. This should not be a change for viewing in Jupyter notebook but fixes opening the svg data in e.g. inkscape where the default font is different and the boxes aren't the right size
 - Don't display virtual tags (for noise) xref quantumlib#2905

* Support multi-qubit measurement in plot_state_histogram (quantumlib#3054)

* Add generated code for batch.proto (quantumlib#3086)

Review: @dstrain115

* Fix flakes in random_circuit_test (quantumlib#3084)

* Fix Quirk import extra gates bug

* Move image files to docs/images/

* Move unneeded site assets

* Move Sphinx templates, Makefile, conf, and init

* Added api docs gen (copy from TFQ). (quantumlib#3089)

* [SVG] Noise hack and font fix (quantumlib#3076)

 - Specify the font. This should not be a change for viewing in Jupyter notebook but fixes opening the svg data in e.g. inkscape where the default font is different and the boxes aren't the right size
 - Don't display virtual tags (for noise) xref quantumlib#2905

* Support multi-qubit measurement in plot_state_histogram (quantumlib#3054)

* Add generated code for batch.proto (quantumlib#3086)

Review: @dstrain115

* Fix flakes in random_circuit_test (quantumlib#3084)

* Added api docs gen (copy from TFQ).

* updated site_path prefix.

* removed pre-built.

* rename.

* move to dev_tools.

* removed __future__ and formatting.

* yet more formatting.

Co-authored-by: Matthew Harrigan <[email protected]>
Co-authored-by: Animesh Sinha <[email protected]>
Co-authored-by: Matthew Neeley <[email protected]>
Co-authored-by: Dave Bacon <[email protected]>

* Move doc tools to /dev_tools/docs/

* Move examples index into tutorials/

* Move basics tutorial into tutorials/

* Encode URL bracket so not mistaken for template.

* Update references to moves docs and image files.

* Update references to docs_coverage_test.py

* Format py with check/format-incremental --apply

* Move Sphinx RST files

* Add _index.yaml for site

* Add initial _book.yaml for leftnav

* Manually format docs/_sphinx/conf.py for CI

* Update references to moved api.rst file

* Move docs/_sphinx to dev_tools/docs/sphinx

* Update README.rst path for snippets_test.py

* Move run_doctest to dev_tools/docs/

* Move build-docs.sh to dev_tools/docs/sphinx/

* Update references to build-docs.sh

* Remove build_docs CI test

Co-authored-by: Matthew Harrigan <[email protected]>
Co-authored-by: Animesh Sinha <[email protected]>
Co-authored-by: Matthew Neeley <[email protected]>
Co-authored-by: Dave Bacon <[email protected]>
Co-authored-by: Matteo Pompili <[email protected]>
Co-authored-by: MichaelBroughton <[email protected]>
tonybruguier pushed a commit to tonybruguier/Cirq that referenced this pull request Aug 23, 2020
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.

Support multi-qubit measurements in cirq.plot_state_histogram
5 participants