quantumlib / Qualtran

Qᴜᴀʟᴛʀᴀɴ is a Python library for expressing and analyzing Fault Tolerant Quantum algorithms.
https://qualtran.readthedocs.io/en/latest/
Apache License 2.0
173 stars 40 forks source link

Title boxes for OnEach bloq are broken #990

Closed fdmalone closed 4 months ago

fdmalone commented 4 months ago

956 removed titles from single qubit gates but this means on each now renders like:

Screenshot 2024-05-23 at 9 34 53 AM

Is the solution here to just say goodbye to $H^{\otimes N}$

mpharrigan commented 4 months ago

OnEach's wire_symbol method must be updated. The title text should be removed and the wire symbol can use either str(self.gate) otime N or self.gate.wire_symbol(Register('q', QBit())) otime N.

fdmalone commented 4 months ago

Maybe I'm missing something but the state of affairs is a little odd:

Screenshot 2024-05-28 at 9 36 35 AM <--- shouldn't have a title but does? Screenshot 2024-05-28 at 9 36 40 AM <--- looks fine Screenshot 2024-05-28 at 9 37 29 AM <--- show_bloq now has no information, musical score looks ok.

fdmalone commented 4 months ago

I know the XGate got missed in #956 but having show_bloq without any bloq information seems wrong?

fdmalone commented 4 months ago

Should we be doing:

diff --git a/qualtran/drawing/graphviz.py b/qualtran/drawing/graphviz.py
index 281571e1..54a46a33 100644
--- a/qualtran/drawing/graphviz.py
+++ b/qualtran/drawing/graphviz.py
@@ -377,7 +377,9 @@ class PrettyGraphDrawer(GraphDrawer):
         if isinstance(binst.bloq, (Split, Join)):
             return ''
         # This wire symbol should always be a text element
-        wire_symbol_title = binst.bloq.wire_symbol(reg=None).text  # type: ignore[attr-defined]
+        wire_symbol_title = str(
+            binst.bloq
+        )  # binst.bloq.wire_symbol(reg=None).text  # type: ignore[attr-defined]
         if not wire_symbol_title:
             return ''
         return f'<font point-size="10">{html.escape(wire_symbol_title)}</font>'

Is this part of a larger issue @mpharrigan / @dstrain115 ?

dstrain115 commented 4 months ago

This is probably because I was not clear exactly how these are rendering and screwed up the refactor. Can you help fix it?

fdmalone commented 4 months ago

Sure yeah, I'm not sure what the state of bloq.__str__ is, so I'll hold off for confirmation before fixing things whole sale.

fdmalone commented 4 months ago

@mpharrigan should this go in before release?

mpharrigan commented 4 months ago

I'll take this