-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add new Quantikz rendering modules and enhance existing functions for visualization #7354
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
base: main
Are you sure you want to change the base?
Conversation
Ideally, we should have some tests, at least for 'circuit_to_latex_quantikz.py' |
Anyone to help clean up and merge in? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting closer!
# ============================================================================= | ||
def render_circuit( | ||
circuit: circuits.Circuit, | ||
output_png_path: Optional[str] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cirq recently managed to update past python 3.10 so we can convert this to "str | None". We can also use dict instead of Dict below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really great, @zlatko-minev! A few requests:
(1) It should have a native understanding of cirq.GridQubit
s and use those as the labels for the qubits.
(2) It should have an option to specify the qubit order, as in circuit.to_text_diagram(qubit_order=qubits)
.
(3) The following circuit renders in a funny way such that it looks like there is a 4-qubit gate, with the CZ gates overlapping:
(1, 1): ───────────H───@───PhXZ(a=0.5,x=0.5,z=0)───
│
(5, 8): ───H───@───X───@───PhXZ(a=-1,x=1,z=0)──────
│
(4, 1): ───H───@───H───@───I───────────────────────
│
(2, 1): ───────────H───@───H───────────────────────
Minor comment:
This does not distinguish between cirq.M(*qubits, key='m)
and cirq.Moment(cirq.M(q, key='m') for q in qubits)
whereas these should be treated differently.
See this colab.
Feature(vis): Enhance circuit_to_latex_render and GIF creation This PR introduces cirq.vis.render_circuit and create_gif_from_ipython_images. The functionality is showcased and described here: in this Jupyter notebook https://drive.google.com/file/d/1KXQjBmQ9rNUjZHJ6QuGolthLN19_K9yK/view?usp=drivesdk (colab: https://colab.sandbox.google.com/drive/1KXQjBmQ9rNUjZHJ6QuGolthLN19_K9yK) Notes on render_circuit: * Performs upfront checks for pdflatex and pdftoppm executables. Issues informative warnings and disables relevant functionality if they are not found, preventing unexpected failures. * If pdflatex fails and an output_tex_path is specified, the problematic .tex file is copied to the specified path for easier user inspection. Note: this is a fork of PR quantumlib#7354
Pushed some changes here that fix all the CI errors and also fixes (1) and (2) from Eliott's comments. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7354 +/- ##
========================================
Coverage 99.37% 99.37%
========================================
Files 1082 1087 +5
Lines 96691 97188 +497
========================================
+ Hits 96086 96583 +497
Misses 605 605 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @dstrain115! Indeed, my comments (1) and (2) are resolved. My comments about two-qubit gates within a moment plotting on top of each other and no distinction between multi-qubit and single-qubit measurements remain, but I'm fine if we want to say that is out of scope for this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The multi-qubit measurement looks great! For the two-qubit operations in the same moment, Cirq's other rendering tools put a bar and under them to indicate that they are in the same moment, like this:
┌──┐
(1, 1): ───────────H────@─────PhXZ(a=0.5,x=0.5,z=0)───
│
(2, 1): ───────────H────┼@────H───────────────────────
││
(4, 1): ───H───@───H────┼@────I───────────────────────
│ │
(5, 8): ───H───@───X────@─────PhXZ(a=-1,x=1,z=0)──────
└──┘
or

Would it be hard to add something like that? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, this is maybe less important and doesn't need to be addressed in this PR, but circuits with classical control, e.g. the first example on this page, are not supported. It throws the error
Error generating LaTeX document: Only GateOperations are supported H(q(1)).with_classical_controls(a)
if I try rendering the circuit from that example, which should look like
0: ───H───M────────────────
║
1: ───────╫───H───M('b')───
║ ║
a: ═══════@═══^════════════
Hi Mike, thanks for finding these. It would be great if someone from the cirq team has the time at the moment to push these small edits over. |
The referenced names in `cirq.ops` module exist if Cirq is operational.
Drop `_HAS_IPYTHON` constant and its dependent branching.
Out of scope, can be done with `pillow` independently of cirq.
Leave few instances that would be otherwise flagged by mypy.
And was guarded by `no cover` coverage markup.
Executed ruff check --select UP045 --fix
@zlatko-minev Just to follow up on the status (although you may have inferred it from the activity on this PR): Pavol is actively working on getting the code into shape for incorporation into Cirq. |
Glad to hear, thank you, @mhucka and @pavoljuhas |
Adopt common pattern `style_pow = f"{style}_pow"`
When rendering fails due to timeout or some other uncaught exception it is better to report that rather than silently return None.
It is better to raise exception and stop for critical errors such as (1) failure to create CircuitToQuantikz converter (2) error in generate_latex_document (3) writing temporary or output files which should be writable if provided
Also update to pragma spelling prevalent in other cirq sources.
Also remove the unnecessary `__all__` list for starred import.
… docstring Tweak LaTeX generation and the expected output so they match in the module docstring.
- Change `test_qcircuit_pdf` to compile PDF file - Install LaTeX and latexmk for the CI jobs Prerequisite for tests in #7354
Fix import path. `quantikz_options` should have no brackets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel the code is in a bit early stage to be eligible for
cirq.vis
, which would imply stable API, so I moved it
to cirq.contrib.quantikz
. There were some anti-patterns such
as too many try-except blocks (which could mask real bugs),
unnecessary hasattr
checks, incomplete test coverage,
render_circuit
function which takes care of too many
things with return values of different meanings and types.
I made some updates to the code which are perhaps easiest to review
commit-by-commit starting from ad58ffb87a.
I think it is in a shape that is OK for contrib, but
I'd like to have a final check by someone else before the merge.
Thank you @zlatko-minev for the contribution and apologies about long-delay in this review.
@zlatko-minev - can you please give this a quick look if this is OK to merge from your side? |
Feature(vis): Enhance circuit_to_latex_render and GIF creation
This PR introduces
cirq.vis.render_circuit
andcreate_gif_from_ipython_images
.The functionality is showcased and described here: in this Jupyter notebook
https://drive.google.com/file/d/1KXQjBmQ9rNUjZHJ6QuGolthLN19_K9yK/view?usp=drivesdk
(colab: https://colab.sandbox.google.com/drive/1KXQjBmQ9rNUjZHJ6QuGolthLN19_K9yK)
Notes on
render_circuit
:* Performs upfront checks for
pdflatex
andpdftoppm
executables. Issues informative warnings and disables relevant functionality if they are not found, preventing unexpected failures.* If
pdflatex
fails and anoutput_tex_path
is specified, the problematic.tex
file is copied to the specified path for easier user inspection.Example outputs:
