-
Notifications
You must be signed in to change notification settings - Fork 342
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
Overhaul Quil to LaTeX circuit generation #1074
Conversation
The previous routine was not very usable, and generated ugly stuff like: With the update, this renders as Attached is an example of the usage of this (showing basic functionality, showing the use of custom settings, and showing the new |
The original has a drop-shadow. I dunno if I can approve not having a drop-shadow. |
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.
Neat! Looks like there is a missing dataclasses import according to the test errors.
b3aaf68
to
30e58ee
Compare
examples/LaTeX Circuits.ipynb
Outdated
} | ||
], | ||
"source": [ | ||
"prog = Program(\"X 0\\n CNOT 0 1\\n H 1\")\n", |
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.
use +=
to build up programs -- if its annoying to use +=
then we should open an issue to fix whatever your pain points are (maybe the pragma and dagger?)
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've changed the first two examples to avoid using escape characters etc. Those are built using the gate objects, e.g. Program(X(0), CNOT(0,1), H(1))
. For the last example I'm going to keep it as a big multiline string -- this is how it came to me (I generated it from the compiler), and I don't think anything would be improved by changing this.
examples/LaTeX Circuits.ipynb
Outdated
], | ||
"source": [ | ||
"# extra kwargs are passed are passed straight to `IPython.display.Image`\n", | ||
"display(prog, width=300)" |
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 gives me an obscure error:
---------------------------------------------------------------------------
RuntimeError Traceback (most recent call last)
<ipython-input-3-6f822dbd0b9c> in <module>
1 # extra kwargs are passed are passed straight to `IPython.display.Image`
----> 2 display(prog, width=300)
~/code/rigetti/github/pyquil/pyquil/latex/ipython.py in display(circuit, settings, **image_options)
40 result = subprocess.call([pdflatex_path, "-output-directory", tmpdir, texfile.name])
41 if result != 0:
---> 42 raise RuntimeError("pdflatex error")
43
44 png = os.path.join(tmpdir, 'diagram.png')
RuntimeError: pdflatex error
which I see in my jupyter notebook logs is actually:
! Package tikz Error: I did not find the tikz library 'quantikz'. I looked for
files named tikzlibraryquantikz.code.tex and pgflibraryquantikz.code.tex, but n
either could be found in the current texmf trees..
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.
we should add some good documentation for installing all the requirements for this module
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.
Adding sphinx docs, and also adding a bit more care to the error reporting here.
Also, need to update the changelog -- I think this warrants the "Announcements" section. |
There are two open features left to implement:
These could be part of this PR or could be part of a separate one (I think the latter is fairly easy to implement, but am running out of time for this today). |
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.
some more stuff
still need to update changelog, and I think the right alignment should be done for this PR. RESET and classical feedback control can definitely be another PR |
f47da60
to
25aa1c9
Compare
99003e6
to
8a33153
Compare
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 great
Description
This is an overhaul of the previous code generating LaTeX circuits. I've got an example notebook included in the the examples directory. This resolves #1023.
Checklist
auto-close keywords.
including author and PR number (@username, gh-xxx).