-
Notifications
You must be signed in to change notification settings - Fork 51
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
Draw latex diagrams for bloqs using QPIC #908
Conversation
@@ -84,3 +86,12 @@ def show_flame_graph(*bloqs: 'Bloq', **kwargs): | |||
"""Display hiearchical decomposition and T-complexity costs as a Flame Graph.""" | |||
svg_data = get_flame_graph_svg_data(*bloqs, **kwargs) | |||
IPython.display.display(IPython.display.SVG(svg_data)) | |||
|
|||
|
|||
def show_bloq_via_qpic(bloq: 'Bloq', width: int = 1000, height: int = 400): |
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 should likely just be a different argument for type
in show_bloq
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.
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 consider this to still be pretty experimental so wanted to keep it separate, but I can add it to show_bloq
as well if you think it's fine.
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.
well let's not make it the default yet, but the "spirit" of the show functions is to provide the simplest possible entry point to getting something to show up in a notebook. I think the type
parameter is perfect for this case
QPIC is not a dependency of Qualtran and must be manually installed by users via | ||
`pip install qpic`. |
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.
why?
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.
License issues. QPIC has a GPL-3.0 license license which is restricted but I confirmed with the open source team that having a dependency like this (where we don't explicit depend on the tool but can produce output the tool can consume) is fine.
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.
do you want to make a note that you can install it with pip but this module will invoke the qpic executable via subprocessing?
output_file: Optional path to the output where generated diagram should be saved. Defaults to | ||
qpic_file.with_suffix(f".{output_type}") | ||
output_type: Format of the diagram generated using qpic. Supported output types are one of | ||
['qpic', 'tex', 'pdf', 'png'] |
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.
what happens if output_file extension and output_type conflict?
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 use temporary files with the right extension as intermediate files to store results and move them to the output file when the right file is computed, so it doesn't matter what the extension of the output file is
The font size looks large compared to e.g. the size of control-circles. Is there a way to control the font size? |
Changed the |
@mpharrigan This is ready for another look. |
The diagrams look really nice. I think the slightly smaller font classes them up |
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.
LGTM modulo merging the show function
Adds support to draw latex diagrams for Bloqs using
qpic
.Here are some pictures generated by
Many other improvements can be made. For example we use a square box instead of left / right triangles (by setting
shape=>
orshape=<
in theG
gates) because elongated triangles look bad. The musical score diagrams added by @mpharrigan use an arrow shape of the form=>
where the width of the horizontal part=
scales with length of the text but the head of the arrow>
stays constant. qpic doesn't have an out of the box option to do this but I think we can achieve something similar usingop="valid_tickz_code_to_draw_an_arrow"
where thevalid_tickz_code_to_draw_an_arrow
needs to be figured out. I spent sometime playing around but gave up. We can do this as a follow upAnother potential improvement is the logic in line manager to allocate lines to given Soquets. In many diagrams shown above, the joined lines are placed below the original split lines even when they are equivalent.
cc @mpharrigan @ncrubin @fdmalone