quantumlib / Cirq

A Python framework for creating, editing, and invoking Noisy Intermediate Scale Quantum (NISQ) circuits.
Apache License 2.0
4.24k stars 1.01k forks source link

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

Closed richrines1 closed 5 months ago

richrines1 commented 8 months ago

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)

maffoo commented 8 months ago

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.

richrines1 commented 8 months ago

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)

github-actions[bot] commented 7 months ago

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

xXxH3LIOSxXx commented 6 months ago

This is AL from the CirqCync - please assign to me

richrines1 commented 6 months ago

^ 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?)

vtomole commented 6 months ago

Hey @xXxH3LIOSxXx ,

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

pavoljuhas commented 5 months ago

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

pavoljuhas commented 5 months ago

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

Done - #6560

pavoljuhas commented 5 months ago

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