Skip to content

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

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

Closed
richrines1 opened this issue Jan 10, 2024 · 9 comments
Closed

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

richrines1 opened this issue Jan 10, 2024 · 9 comments
Assignees
Labels
good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. kind/feature-request Describes new functionality no QC knowledge needed Want to contribute to Cirq, but don't know quantum computing? This issue is for you. triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@richrines1
Copy link
Contributor

richrines1 commented Jan 10, 2024

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 richrines1 added the kind/feature-request Describes new functionality label Jan 10, 2024
@maffoo
Copy link
Contributor

maffoo commented Jan 18, 2024

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
Copy link
Contributor Author

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)

Copy link

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

@vtomole vtomole added the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Mar 22, 2024
@pavoljuhas pavoljuhas added triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on no QC knowledge needed Want to contribute to Cirq, but don't know quantum computing? This issue is for you. good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Mar 27, 2024
@xXxH3LIOSxXx
Copy link

This is AL from the CirqCync - please assign to me

@richrines1
Copy link
Contributor Author

^ 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
Copy link
Collaborator

vtomole commented Mar 27, 2024

Hey @xXxH3LIOSxXx ,

Please review #6530 if you can.

@pavoljuhas
Copy link
Collaborator

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

@pavoljuhas
Copy link
Collaborator

pavoljuhas commented Apr 10, 2024

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

Done - #6560

@pavoljuhas
Copy link
Collaborator

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

jselig-rigetti pushed a commit to jselig-rigetti/Cirq that referenced this issue May 28, 2024
…ib#6530)

Use more readable rendering of tagged operations in diagrams.

Fixes: quantumlib#6411
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
…ib#6530)

Use more readable rendering of tagged operations in diagrams.

Fixes: quantumlib#6411
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. kind/feature-request Describes new functionality no QC knowledge needed Want to contribute to Cirq, but don't know quantum computing? This issue is for you. triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Development

No branches or pull requests

6 participants