Circuit image plotter #295
Conversation
…sdk-py into circuit_plotter
|
@diego-plan9 this now checks to see if poppler and pdflatex are installed. if not, it emits a warning and returns no image. |
There was a problem hiding this comment.
Thanks, @ajavadia ! It's looking good, with some tiny inline comments! Eventually it would be great to move to a lighter solution that does not require the extra steps for converting to latex->pdf->png, but in the meantime the solution would work wonderfully!
| @@ -82,6 +77,7 @@ | |||
| QASM_source = Q_program.get_qasm("initializer_circ") | |||
|
|
|||
| print(QASM_source) | |||
| plot_circuit(circuit) | |||
There was a problem hiding this comment.
Mainly for double-checking: are the changes on this example intentional?
There was a problem hiding this comment.
Yeah I just wanted to enrich one of the existing examples with the new circuit drawing capabilities.
qiskit/tools/visualization.py
Outdated
| from collections import Counter, OrderedDict | ||
| from functools import reduce | ||
| from io import StringIO | ||
|
|
||
| from PIL import Image, ImageChops |
There was a problem hiding this comment.
Following up on the idea of making the feature "optional": can we also make pillow optional, as it will not be used unless the non-python dependencies are not installed? This would involve:
- making this import "conditional-ish", either using something similar to how projectq is handled, or importing it inside the function that uses (they are currently two, but with a bit of moving things around it seems it might be simple to reduce it to one)?
- remove it from
requirements.txt(ideally, marking is as an "extra" dependency insetup.py.in- although we are not really using theextras_requireofficially, it might be a good time to also addprojectqto that dict if you feel inclined to do so) - update the docstrings and the
tryblock to be aware of pillow
There was a problem hiding this comment.
I implemented your 1st and 3rd bullets.
Regarding the 2nd - I agree with you that pillow is only required for circuit drawing, similar to the other dependencies introduced here. But adding it as extras_require would mean that it does not get installed automatically. I think this is not a heavy dependency (unlike pdflatex and poppler); so wouldn't it be better to remove the burden of manual installation on the user? I also imagine pillow will be used in any eventual, light-weight solution for circuit drawing (as indicated in Ismail's original issue #235).
For projectq, I agree -- it is tangential to the SDK. Added to extras_require
qiskit/tools/visualization.py
Outdated
| Note: Requires poppler installed (to convert pdf to image) | ||
| """ | ||
| filename = 'circuit' | ||
| tmpdir = 'tmp/' |
There was a problem hiding this comment.
Can we use tempfile.TemporaryDirectory or similar for creating the temporary directory? It's quite handy for these cases, as it will make sure that the folder is created in a safe place and also takes care of deleting it if using it as a context manager.
There was a problem hiding this comment.
Yes that's much better. Done!
|
Thanks for your nice detailed comments @diego-plan9. I addressed your suggestions. See further comments inline. |
|
Thanks, @ajavadia! I added a small fix to In regards to the optional dependencies vs regular dependencies: while making This would help us with mantainability and avoiding to have a number of dependencies installed that are not really in use, being some sort of "good citizen" in that regard. Still, I see the point of view, and we are still far away from a point where we have an unmanageable number of dependencies - so we can revise later if needed! |
* one more initializer test case * updating version requirement for IBMQuantumExperience * fixing per diego's comments * circuit_drawer() visualization.py, generating PIL images * adding circuit drawing function and embedded example in initialize * added test * linting * more linting * better exception handling in case dependencies not installed. * fixing circuit drawing tests * more linting-_- * fixing per diego's comments * lint * improved exception handling for dependencies pillow, poppler, pdflatex, qcircuit * adding id to drawing basis default * Fix typo in setup.py.in
This addresses Issue #235: drawing quantum circuits locally.
It adds 2 functions to
visualization.py:circuit_drawerreturns a PIL image format of the circuit.plot_circuitis a thin wrapper around the above, which just displays the circuit on screen.The reason for this split is that PIL images cannot be easily inlined in jupyter notebooks, contrary to e.g. matplotlib (and matplotlib did not render these images well). So for Jupyter inlining one would call the 1st function, otherwise the 2nd function.
This PR adds several dependencies:
requirements.txt)Motivation and Context
This will be a nice feature to have, especially for writing tutorials.
How Has This Been Tested?
test/python/test_visualization.py.examples/python/initializer.py.Screenshots (if appropriate):
Types of changes
Checklist: