quantumlib / Cirq

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

cirq.X and cirq.X**(-1) render the same in text diagrams #3597

Open zchen088 opened 3 years ago

zchen088 commented 3 years ago

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

import cirq 
a = cirq.LineQubit(1)   
cirq.Circuit(cirq.X.on(a), cirq.X.on(a)**(-1))  

> 1: ───X───X───

Now, this is sensible because X and -X have the same unitary up to a global phase. However, X and -X are implemented differently when you run them on hardware, and we use this fact sometimes when doing experiments on hardware.

Describe the solution you'd like I think it'd make sense to render the exponent when it's -1, like

> 1: ───X───X^-1───

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

balopat commented 3 years ago

I'm okay with this but would love the @quantumlib/cirq-maintainers to chime in on it. Do we want this to be the default behavior always or should we put it behind a flag (e.g we could have acirq.set_printoptions() method similar to np.set_printoptions) as the default X display has some benefits to it too?

viathor commented 3 years ago

This looks like a requirements conflict. On one hand, from the abstract perspective X and its inverse are identical. On the other, they may be implemented differently on the hardware.

We have introduced tags to carry some extra information to aid hardware implementation (e.g. VirtualTag). Perhaps we can also use tags to encode which "branch" the gate exponent lies on (to distinguish t=..., -3, -1, 1, 3, ... for which X^t are all equivalent)? This would allow unannotated gates to continue to behave as most users would expect.

maffoo commented 3 years ago

Seems like we should fix this in circuit diagrams. The str and repr of cirq.X**-1 do preserve the exponent:

In [1]: repr(cirq.X)                                                                                     
Out[1]: 'cirq.X'

In [2]: repr(cirq.X**-1)                                                                                 
Out[2]: '(cirq.X**-1)'

In [3]: str(cirq.X)                                                                                      
Out[3]: 'X'

In [4]: str(cirq.X**-1)                                                                                  
Out[4]: 'X**-1'
mpharrigan commented 3 years ago

On the other hand, circuit diagrams are really sensitive to long names because everything has to get pushed out. For example: I remember making PhasedXGate render as PhX which is not the str

maffoo commented 3 years ago

@mpharrigan, that's a good point about wanting to be concise. But abbreviated names don't lose any information, whereas eliding exponents to show an "equivalent" gate drops info that is useful when running circuits on hardware (a core cirq use case). I'd suggest we treat circuit diagrams more like repr's that should just show the structure and not try to simplify things too much. If you want to replace X**-1 with X that would be an optimization pass to run before displaying the circuit.

viathor commented 3 years ago

@maffoo I don't think treating X and X-1 qualifies as eliding information because the exponent is periodic. X and X-1 really are the same gate. It is true that a particular hardware implementation may distinguish these two cases, but I am worried that the (in)equivalence induced by two different hardware implementations may differ. For this reason, it may be safer to separate these concerns into layers and have gate objects satisfy the standard gate relationships implied by the structure of the unitary group and tags to add extra information to allow finer-grained distinctions peculiar to various hardware implementations.

I propose to fix repr and str to account for periodicity and to add tags to represent parameter branches.

(BTW: We should probably account for periodicity of gate parameters everywhere in cirq. I think we did the right thing in this respect for the new PhasedFSimGate, but then discovered that FSimGate needed a fix.)

maffoo commented 3 years ago

I guess it depends on what "the same" means. They are certainly equivalent in some senses, but not in others, and in particular not in a sense that we care about for hardware implementations. I suppose one could argue that if we care about the distinction in hardware then we should use cirq.rx instead where cirq.rx(np.pi) and cirq.rx(-np.pi) include global phases and so are clearly "different", but that has its own awkwardness. I'd prefer not to track things like which branch we are on in a tag, especially since we already have an exponent property on the gate itself for this. IMO we should make the gate object as simple as possible and put the "intelligence" about periodicity into code that checks for equivalence, or code that can be explicitly called to "canonicalize" and get an equivalent gate on the principal branch, say. Such canonicalization can be performed as part of various optimization steps, but I'd prefer to make that explicit: don't do it automatically when constructing gate instances, and definitely don't doing when rendering a circuit diagram :-)

viathor commented 3 years ago

Fair enough. I see how not having to use tags for this makes things easier from the practical standpoint.

In any case, we should probably have one consistent approach to dealing with periodicity of gate parameters that can be relied on for all gates, so if we prefer gates to remember the "lifted" (i.e. pre-canonicalization) values then we should probably change the FSimGates (and maybe others that do canonicalization implicitly at instantiation).

viathor commented 3 years ago

Regarding rendering, in the unicode mode we could render X inverse as X† which is merely two characters long instead of five in X**-1.

balopat commented 3 years ago

One more thing to add: Currently we display 0th power, e.g. for X**0: we display ─X^0─. We could just ignore those gates or display an identity gate instead - but we are not doing that and it provides clarity and transparency for the user for debugging their circuit construction.

@viathor I liked the X† idea, but there is a question: should then make that a universal rule for all negative exponents? I.e. X**-3.5 should be displayed as ─X†^3.5─? If not, isn't that just too much special casing and display inconsistency for a couple of characters?

balopat commented 3 years ago

Discussed on Cirq Cynque: consensus is that we should be explicit about the exponent in a numerical way instead of swallowing it here. If the user wants to remove these from the diagrams, they can canonicalize the operations. Also, we don't recommend the dagger sign to minimize the changes to the diagram drawing. cirq.Y, cirq.Z and all other affected gates should follow suit.

daxfohl commented 3 years ago

Would it be useful at all to have a deserializer for the diagrams, and make sure it is round-trip?

balopat commented 3 years ago

Would it be useful at all to have a deserializer for the diagrams, and make sure it is round-trip?

Given that we don't typically use circuit diagrams as inputs I think a deserializer is not something we are looking for currently. If it would be easier to write a circuit diagram than a circuit then maybe, but that's really not the case, I typically prefer to write the python code and generate the diagram instead.

ghost commented 3 years ago

I would like to work on this issue.

Ishan-Gokhale commented 2 years ago

May I work on this issue? I would really like to work on this issue.

vtomole commented 2 years ago

@Ishan-Gokhale I've assigned it to you.