Adding function to create a quirk url from a bloq#1821
Adding function to create a quirk url from a bloq#1821Axel-pappalardo wants to merge 5 commits intoquantumlib:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
…eManager + more testing
|
/gemini review |
There was a problem hiding this comment.
Code Review
This PR adds a new feature to generate Quirk URLs from bloqs. The implementation is mostly correct, but there are several critical issues in SparseLineManager that could lead to runtime errors due to incorrect handling of DanglingT instances. I've provided suggestions to fix these. Additionally, there are opportunities to improve code quality and portability in composite_bloq_to_quirk by refactoring the line removal logic and using the webbrowser module instead of a hardcoded subprocess call.
| if open_quirk: | ||
| subprocess.run(["firefox", url], check=False) |
There was a problem hiding this comment.
Using subprocess.run with a hardcoded "firefox" is not portable. The webbrowser module is the standard Python way to open a URL in the user's default browser, which is more portable and robust.
| if open_quirk: | |
| subprocess.run(["firefox", url], check=False) | |
| if open_quirk: | |
| import webbrowser | |
| webbrowser.open(url) |
There was a problem hiding this comment.
Tried the change but it links to a quirk error page with an empty circuit
As per the title, This adds a function which yields a quirk url from a qualtran bloq.
The function isn't perfect, and runs into many limitation:
handled_operationsdict), operations not handled are simply ignored.It works by using the musical score of the bloq to look through the operations column per column and thus can use a
LineManager.I also implemented a default
LineManagerSparseLineManagerto avoid an issue where if 2 different qubit lines share the same horizontal line in a musical score (which can happen withPartition) they would be put on the same line in Quirk (this is because to determine Quirk coordinates for an operation we only look at its coordinates in the musical score).SparseLineManageravoid this issue by reserving the slots used by any line that ends after aPartitionand freeing them only if the adjoint of thatPartitionappears again. (for example, if I join 3 QBit into a QAny(3), the slots used by the 3 QBit are reserved until I split the QAny(3) back into 3 QBit).An example generated with that code:
Lookup table
It could still have some issue with certain Bloq, but making specific
LineManagershould helpLet me know what you think