-
Notifications
You must be signed in to change notification settings - Fork 177
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
Circuit diagrams for unitary quantum kernels in LaTeX #1723
Circuit diagrams for unitary quantum kernels in LaTeX #1723
Conversation
CLA Assistant Lite bot All Contributors have signed the CLA. |
I have read the Contributor License Agreement and I hereby accept the Terms. |
I am currently building the latex reference output for testing, to match this string output: cuda-quantum/unittests/integration/draw_tester.cpp Lines 66 to 86 in 1c76ab2
This is the current draft:
which produces I think that My svg viewer shows weird triangles for the numerical dots (they aren't there when I run @bettinaheim @amccaskey @boschmitt does this look somewhat like the thing you'd like? |
I think this PR fulfills the requirements stated in issue #1638. However, I would like to do some more notebook integration.
Opinions are highly appreciated! |
Hi @freifrauvonbleifrei, thanks for all the work put into this. I noticed that you are using C++20 concepts in the code. We also use it, however, you need to provide an alternative so that the code can also compile with C++17. For example: cuda-quantum/runtime/cudaq/algorithms/sample.h Lines 242 to 249 in b522e18
|
Hi @boschmitt , thank you for having a look. I was not aware that C++17 compatibility would be required, and it is especially surprising to me that other places in the code use concepts unguarded:
How are these other parts ensured to be compiled with C++20? I can add some guards for sure, but I would like to make sure this is what is needed in the project, as readability would really suffer. |
starting to separate the layering from the drawing
…m_trace to separate the layering from the drawing
(new function string_diagram_from_trace)
(cherry picked from commit 96d5731)
and delegating draw overload specified in issue #1638
using invocable concept
which can take a kernel without passed arguments
(format selection by string as in the C++ interface described in issue #1638)
for svg display in Jupyter notebooks
4825229
to
60a6ed6
Compare
I was not sufficiently precise in my observation. The thing is: not all parts of the codebase have the same requirements regarding C++17 support. We need C++17 support on the public headers, i.e., the headers users would import (directly or indirectly) to develop their quantum applications. (In other words, to build the project, one needs a C++20 capable compiler. However, to use it, you only need C++17.) The code you showed is not part of the public API and is not installed (distributed with CUDAQ). Thus, there is no need to comply with the C++17 requirement. |
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.
Nice! Overall this looks very nice.
Let's follow up in a separate PR for the C++ 17 support in the public headers, so that we can stick to the Unitary Hack time lines.
Command Bot: Processing... |
Command Bot: Processing... |
Command Bot: Processing... |
CUDA Quantum Docs Bot: A preview of the documentation can be found here. |
Add LaTeX circuit diagrams to the cudaq draw() function.
Description
this PR addresses #1638