`quippername` should be treated more consistently with gate names for other export formats

[dlyongemallo]

In circuit/gate.py, the gate names used for exporting to other formats, qc_name and qasm_name (qasm_name_adjoint), are clearly declared as class variables with a value of undefined for the base Gate, with each subclass expected to override these values. The to_qc and to_qasm functions use qc_name and qasm_name[_adjoint], respectively. However, quippername behaves differently, in that it is undefined at the base Gate level, and the to_quipper function uses name if quippername is not explicitly set. This violates the principle of least astonishment.

In particular, this has resulted in a bug whereby if one constructs a ParityPhase gate, then calls to_quipper on it, the program crashes (due to ParityPhase not having a target attribute, having one called targets instead), rather than the expected behaviour of raising a TypeError as seems to be intended by the code. This can be fixed by setting quippername to undefined for ParityPhase, but it would be much better to set this as the class variable's default value. (This will require explicitly setting quippername for those gates for which it is currently relying on the fallback to name.)

(Incidentally, I also noticed that the FSim gate has a qsim_name which isn't used elsewhere. The documentation seems to imply that there should be a to_qsim function, but this doesn't exist. Is it just not written yet? In any case, the same comment applies to qsim_name as quippername in terms of making it clearly a class variable with a default of undefined.)

[jvdwetering]

With regards to quippername, I think I made this shortcut because I was lazy. I agree that your solution would be more legible. With qsim I'm not sure what the idea is. I believe @akissinger implemented this when he was doing some hacking with the Google supremacy circuits.

[dlyongemallo]

I'm not sure if this is a bug or something I just don't understand. The CX class inherits from the CZ class. I see in the git history that it previously had its qasm_name and qc_name set to undefined, which was subsequently removed. Consequently, it inherits these names from CZ. If I create a CX gate and call to_qc and to_qasm, the result is Z and cz respectively. Should these not be X and CX? I do see that for some of the other gates which aren't supported by the export format, that a conversion implicitly takes place, but it doesn't seem that's what's happening here.

Repro case:

cx = CX(0, 1)
print(cx.to_qasm())
print(cx.to_qc())
print(cx.to_quipper())

Output:

cz q[0], q[1];
Z q0 q1
QGate["X"](1) with controls=[+0] with nocontrol

Incidentally, what is the difference between the CX and CNOT classes? I see from to_graph that the CNOT is a z-spider on the control and x-spider on the target, which is what I expect, but the CX is x-spiders on both. Furthermore, the CNOT gate is exported as follows:

cx q[0], q[1];
Tof q0 q1
QGate["not"](1) with controls=[+0] with nocontrol

I'm not sure if there's some technical difference between controlled-NOT and controlled-X in pyzx, but as I understand it they're usually synonymous , so both should have a to_qasm output of cx. The documentation here says "These circuits [are] composed of CNOT, CX and ZPhase gates, which can be directly translated to phase gadgets in ZX", which suggests CNOT and CX are distinct things, but there's no mention of CX in the code that actually generates the phase polys that I can see.

I don't know the qc format, but is Tof the correct output? I'd expect that to be a 3-qubit gate. As well, should the to_quipper outputs be different for CX and CNOT?

(This comment is going a bit off-topic from quippername, but it's related in the sense it's difficult for a new reader of the code to see what the expected output of the to_quipper function should be.)

[dlyongemallo]

For completeness:

pp = ParityPhase(0, 0, 1)
print(pp.to_qasm())
print(pp.to_qc())
print(pp.to_quipper())

Output:

cx q[0], q[1];
rz(0.0*pi) q[1];
cx q[0], q[1];
Tof q0 q1

Tof q0 q1
Traceback [...]
AttributeError: 'ParityPhase' object has no attribute 'target'. Did you mean: 'targets'?

Setting the quippername for ParityPhase to undefined (explicitly, or through inheritance) changes this to:

QGate["not"](1) with controls=[+0] with nocontrol
QRot["exp(-i%Z)",0.0](1)
QGate["not"](1) with controls=[+0] with nocontrol

As I'm not familiar with the expected outputs, please let me know if these look correct. (For example, should the newline for the to_qc output be there?)

[jvdwetering]

The CX gate is the X-controlled X gate', which does not have a standard name (it is a CNOT where the control wire is conjugated by Hadamards). I agree that this is confusing as CX is usually a synonym for CNOT. A better name would be XCX for instance. With the names not being defined for this class is indeed a bug. Theqcformat supports Toffoli's with any number of controls, so a single controlled Toffoli is a CNOT.qc` is really just a format for typesetting quantum circuits, and because single-controlled Toffoli's look exactly like CNOTs, this is fine.

I don't know about the newline in the to_qc output, that looks wrong to me.

[dlyongemallo]

Regarding the newline in the to_qc output: when the phase is 0 on a ZPhase or XPhase gate, calling to_quipper produces an empty string. If such a gate is produced by to_basic_gates, as for example in the middle of a phase gadget, this results in to_quipper producing an empty line (i.e., between the two ladders of the phase gadget). Since a 0 phase is a noop, I guess this is the correct behaviour as presumably the newline is a noop for quipper. It only looks strange cosmetically.