Cirq icon indicating copy to clipboard operation
Cirq copied to clipboard

use `str` instead of `repr` for tags in circuit diagram

Open richrines1 opened this issue 1 year ago • 3 comments

Is your feature request related to a use case or problem? Please describe.

currently tags are displayed in circuit diagrams via their repr (implicitly via str(list(tags))). This can lead to some unruly or hard-to-read circuit diagrams, e.g.:

1: ───X^0.5───Rz(0.928π)[cirq.VirtualTag()]───X^0.5───Rz(π)[cirq.VirtualTag()]───────@──────────────────────────────────
                                                                                     │
2: ───X^0.5───Rz(0.072π)[cirq.VirtualTag()]───X^0.5───Rz(-0.5π)[cirq.VirtualTag()]───X───Rz(-1.5π)[cirq.VirtualTag()]───

this could be cleaned up by overriding tag.__repr__, but usually not without breaking the expectation that eval(repr(tagged_op)) == tagged_op

Describe the solution you'd like

it seems like it'd be much more natural (and easier to customize) if the circuit diagram drawer to instead use the tags' strings, e.g.:

1: ───X^0.5───Rz(0.928π)[<virtual>]───X^0.5───Rz(π)[<virtual>]───────@──────────────────────────
                                                                     │
2: ───X^0.5───Rz(0.072π)[<virtual>]───X^0.5───Rz(-0.5π)[<virtual>]───X───Rz(-1.5π)[<virtual>]───

What is the urgency from your perspective for this issue? Is it blocking important work?

P3 - I'm not really blocked by it, it is an idea I'd like to discuss / suggestion based on principle

(happy to work on this if it seems reasonable)

richrines1 avatar Jan 10 '24 20:01 richrines1

Another possibility would be to allow tags to implement some protocol method to produce a more compact representation for circuit diagrams. Something like _circuit_diagram_info_ itself allows for gates.

maffoo avatar Jan 18 '24 23:01 maffoo

that would definitely be the nicest solution, in that it could also support e.g. CircuitDiagramInfoArgs.use_unicode_characters when generating the symbol

i still think it would make sense to fall back on str(tag) instead of repr for tags which don't implement _circuit_diagram_info_ though - if nothing else this is what happens for gates:

class Foo(cirq.Gate):
    def _num_qubits_(self): return 1
    def __str__(self): return "<str>"
    def __repr__(self): return "<repr>"

print(cirq.Circuit(Foo().on(cirq.q(0))))  # prints "0: ───<str>───"

It also seems more in keeping with the goal of the circuit diagram generally - usually the point of implementing __str__ for a particular object is to provide a more human-readable visual representation than its __repr__ (which is generally supposed to be unambiguous)

richrines1 avatar Jan 24 '24 19:01 richrines1

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days

github-actions[bot] avatar Feb 26 '24 00:02 github-actions[bot]

This is AL from the CirqCync - please assign to me

xXxH3LIOSxXx avatar Mar 27 '24 17:03 xXxH3LIOSxXx

^ i actually have a working version of this locally i can push :)

it just uses __str__ instead of __repr__ though - if you want to also check for tag._circuit_diagram_info_ as suggested above that would have to be added (maybe as a separate pr?)

richrines1 avatar Mar 27 '24 18:03 richrines1

Hey @xXxH3LIOSxXx ,

Please review https://github.com/quantumlib/Cirq/pull/6530 if you can.

vtomole avatar Mar 27 '24 20:03 vtomole

Thank you @richrines1 and @xXxH3LIOSxXx for helping to improve this. I am assigning to richrines1 as he has already submitted a PR.

pavoljuhas avatar Apr 09 '24 20:04 pavoljuhas

csynkque meeting - TODO @pavoljuhas - create a new issue for supporting the _circuit_diagram_info_ protocol

Done - #6560

pavoljuhas avatar Apr 10 '24 17:04 pavoljuhas

Continued as #6560. Anyone interested (@xXxH3LIOSxXx, @richrines1) , please feel free to comment there if you'd like to take it on. Thank you!

pavoljuhas avatar Apr 10 '24 23:04 pavoljuhas