Skip to content
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

Support _circuit_diagram_info_protocol_ for customized rendering of tags in circuit diagrams #6560

Open
pavoljuhas opened this issue Apr 10, 2024 · 6 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 there is consensus amongst maintainers that this is a real bug or a reasonable feature to add

Comments

@pavoljuhas
Copy link
Collaborator

This is a follow-up to #6411 which introduced str() as a default representation of tags in a text circuit diagrams.
This should be kept as a default method, but we would like to also support the _circuit_diagram_info_protocol_
for tags. _circuit_diagram_info_protocol_ - if defined - would produce a specialized string for rendering the tag object in circuit diagrams.

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

P2 - we should do it in the next couple of quarters

@pavoljuhas pavoljuhas added kind/feature-request Describes new functionality good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. no QC knowledge needed Want to contribute to Cirq, but don't know quantum computing? This issue is for you. triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add labels Apr 10, 2024
@xXxH3LIOSxXx
Copy link

@richrines1 @maffoo - FYI - this was the spin off from maffoo's comment

We talked about it on the sync today briefly. My curiosity/concerns were:

  • In practice how many tags are anticipated to be rendered?
  • Since a tag can be any hashable is there any gain from having the two existing tag classes implement something like _circuit_diagram_info_ as maffoo suggested, when tags could just as easily be pure strings (i.e. inconsistent in appearance)
  • What value in practice is there for representing arbitrary tags in the diagrams? As opposed to say just a symbol representing tags exist at a given point/position, or requiring them to be specific tag objects which must implement a method defining how they're rendered.

@SarthakNikhal
Copy link

Can I help with this issue

@xXxH3LIOSxXx
Copy link

My thoughts from the past few weeks

  • Users can currently create any number of tags
  • Those tags can be any hashable type, and don't necessarily have to be one of the tag objects that exist and can therefore contain some kind of _circuit_diagram_info_ information
  • Because of these points - is there any gain by designing something specific for the two existing objects, - or does it make more sense to generalize such that:
    • If any tags are present that some sort of symbol is appended to whatever operation is tagged so that reviewers of the diagrams understand there may be additional factors at play on any given operation
    • This train of thought makes me feel like, the only relevant abstractions of tags therefore may be something like:
      • When general (non-object) tags exist on an operation, a general marker is used
      • When specific tags exist that have meaning beyond user convenience (e.g. virtual operations), a specific tag is used and perhaps the display of that is supplied through something like _circuit_diagram_info_
  • The next question comes down to relevance of these display elements in future abstractions. For example when a series of moments or operations are squashed into another operation. Is the tag information relevant to a non-creator user of such an operation? For example:
    • A user may create general tags for some personal convenience or relevance to them, but maybe if they consolidate and make available this consolidated operation in some way to other users, perhaps the tags are irrelevant to them and so abstractions of general tags may not be useful
    • In the same case, but where specific tags which have mechanical implications like virtual tags indicating virtual or external operations which may be scientifically relevant, perhaps those decorations need to float up through the abstraction so that consumers are aware and can be informed.
  • What are even the use-cases for general tags in the existing user base? Maybe it makes sense to make tags only be specific and relevant and therefore implement specific methods for supplying graphical decorations..

What we covered in today's sync

  • We don't need to restrict users use of tags, if they want to make very lengthy, general tags, then that's up to them
  • The purpose of the ascii diagrams is as a visual aid to a user constructing either operations or complete circuits, not necessarily as a valuable resource for complex circuits or situations where the length makes the value of the graphic diminish proportionally
  • There is no set boundary defined from a use perspective where circuit diagrams should be abstracted to facilitate displaying really complex circuits. In the same vein as 'you can make whatever tags you want' - 'you can refer to as crazy an ascii diagram as you want' (i.e. - it either helps you or it doesn't).

Grain of salt on these 3 points, these are my own personal interpretation of the feedback obtained during the call. @pavoljuhas - feel free to correct any potential misunderstandings on my part on those summary points!

My current proposal for a way to proceed

  • Create a process for identifying general tags on operations.
    • Have that process supply a generalized symbol to append to whatever operation contains those general tags
    • Have a toggle for displaying that decoration on the circuit diagram or not
  • Implement a specific symbol for the specific tag objects (2) that exist today to similarly append to operations containing those tags
    • Ensure that these specific symbols can 'float up' to parent operations as they indicate actual non-arbitrary mechanisms performed
    • Create a toggle for displaying these specific tag decorations in the circuit diagrams that is usable at different levels of abstraction and which can differentiate between specific and general tags

Looking forward to anyone's thoughts on these points!!

@xXxH3LIOSxXx
Copy link

Can I help with this issue

./wave @SarthakNikhal - I think the door's open to anyone (if I'm any indication of that :) ) - and I would love a partner in crime on this deeper-than-it-first-appeared concept! Could you have a read over my last post and let me know what you think? If it makes sense to meet to discuss rather than bloat the issue dialogue lets figure out a way to do that.

@pavoljuhas - In cases where we want to collaborate on this - is the best practice to keep all the dialogue/thoughts here or to try to limit or consolidate updates here? I wasn't sure if my last lengthy post was really desirable or not or if I want to spitball with Sarthak, if that's really welcomed here on the issue itself.

@pavoljuhas
Copy link
Collaborator Author

@xXxH3LIOSxXx - thank you for a detailed summary of discussions thus far.

I feel the best way to go around this is to start with a minimum useful extension and leave more intricate use cases for later when (and if) they prove necessary. As it is the CircuitDiagramInfoArgs class which customizes the rendering of diagrams has include_tags attribute which can be used to show or ignore tag presence.

I think the easiest option is to adjust the TaggedOperation._circuit_diagram_info_ method to check if the present tag objects provide _circuit_diagram_info_ themselves.
If so, the method should return tag rendering in wire_symbols[0] in a similar fashion as done for LineQubit.
For tags that do not have _circuit_diagram_info_ the rendering should fall back to str(tag) as done now.

git grep 'class \S*Tag\b' "*py" ":(exclude)*test.py" shows 5 non-test SomeTag classes.
I feel for now we should leave their rendering as is, but the test-only Tag classes can and should exercise the new capability for the circuit_diagram_info protocol.

@xXxH3LIOSxXx - since you commented first and already showed interest in #6411 -
can you take this on at this minimum initial scope?

Thank you BTW both (@SarthakNikhal) for your offer to help!

@xXxH3LIOSxXx
Copy link

@pavoljuhas - Got it and excited to begin digesting this and working on it!

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 there is consensus amongst maintainers that this is a real bug or a reasonable feature to add
Projects
None yet
Development

No branches or pull requests

3 participants