Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
5k stars 2.32k forks source link

Consistent naming of controlled gates #3633

Closed Cryoris closed 4 years ago

Cryoris commented 4 years ago

As discussed with @ewinston, the names of the controlled gates are inconsistent. For instance, the controlled version of HGate is called CHGate but YGate becomes CyGate. This applies to many others too. We should name them all consistently.

Possible choices would be:

  1. Only the first letter of the gate name is upper case, i.e.

    YGate CyGate RyGate CryGate U3Gate Cu3Gate
  2. The gate names are full uppercase, i.e.

    YGate CYGate RYGate CRYGate U3Gate CU3Gate
  3. The control gates only prepend a C, the gate name is kept, i.e.

    YGate CYGate RyGate CRyGate U3Gate CU3Gate

    Currently the rotations are called RXGate, thus 2. and 3. might be the same. Most rotation gates follow 1., which I personally think looks most aesthetic and fits also with the named gates as Fredkin or Toffoli. However, I think writing all in uppercase is probably the least error-prone method.

What are the thoughts on this?

ajavadia commented 4 years ago

Yes this has been bothering me for some time too.

I'm just casting a vote here, but since each letter has a meaning I think they should all be capitalized (C=Control, R=Rotation, X=X axis):

According to PEP8:

When using acronyms in CapWords, capitalize all the letters of the acronym. Thus HTTPServerError is better than HttpServerError.

Should we also change CnotGate -> CXGate (with a deprecation warning)? What about ToffoliGate -> CCXGate and FredkinGate -> CSwapGate? These class names differ from the filename they are in, and also the corresponding QuantumCircuit.xxx method.

While you're at this maybe you can take care of #3535 too!

Cryoris commented 4 years ago

I agree that changing the name of the 'named' gates to functional names, i.e. ToffoliGate -> CCXGate, especially since the functional names the names of the circuit methods. Defining these gates via their names we only introduce more terms.

An argument for Ry/CRy over RY/CRY would be that the first corresponds more to the mathematical way of writing it as $R_y$. Also this is how the convention is in projectq, cirq and Q# (if that counts as argument for more consistency within quantum software kits 😄).

@ajavadia actually #3535 can be closed, since the crx was implemented by #3600.

ajavadia commented 4 years ago

Sure i'm good with Ry/CRy too.

kdk commented 4 years ago

I agree it'd be good to have consistency here, especially if we can find a pattern that would cover the the full standard gate set.

My vote would be to move to full uppercase gate names (in line with option 2, and @ajavadia 's suggestion above) only for simplicity. e.g. For someone not from a mathematical background, would it be clear why when a YGate is controlled, it becomes CYGate but when it is parameterized, it becomes an RyGate?

Just for reference, here is the current state of affairs (for everything defined in extensions/standard/:

| file                                      | class              | name        | qc._           |
|-------------------------------------------+--------------------+-------------+----------------|
| barrier.py                                | Barrier            | barrier     | barrier        |
| ccx.py                                    | ToffoliGate        | ccx         | ccx, toffoli   |
| ch.py                                     | CHGate             | ch          | ch             |
| crz.py                                    | CrzGate            | crz         | crz            |
| cswap.py                                  | FredkinGate        | cswap       | cswap, fredkin |
| cu1.py                                    | Cu1Gate            | cu1         | cu1            |
| cu3.py                                    | Cu3Gate            | cu3         | cu3            |
| cx.py                                     | CnotGate           | cx          | cx, cnot       |
| cy.py                                     | CyGate             | cy          | cy             |
| cz.py                                     | CzGate             | cz          | cz             |
| h.py                                      | HGate              | h           | h              |
| iden.py                                   | IdGate             | id          | iden           |
| ms.py                                     | MSGate             | ms          | ms             |
| r.py                                      | RGate              | r           | r              |
| rx.py                                     | RXGate             | rx          | rx             |
| rxx.py                                    | RXXGate            | rxx         | rxx            |
| ry.py                                     | RYGate             | ry          | ry             |
| rz.py                                     | RZGate             | rz          | rz             |
| rzz.py                                    | RZZGate            | rzz         | rzz            |
| s.py                                      | SGate              | s           | s              |
| s.py                                      | SdgGate            | sdg         | sdg            |
| swap.py                                   | SwapGate           | swap        | swap           |
| t.py                                      | TGate              | t           | t              |
| t.py                                      | TdgGate            | tdg         | tdg            |
| u1.py                                     | U1Gate             | u1          | u1             |
| u2.py                                     | U2Gate             | u2          | u2             |
| u3.py                                     | U3Gate             | u3          | u3             |
| x.py                                      | XGate              | x           | x              |
| y.py                                      | YGate              | y           | y              |
| z.py                                      | ZGate              | z           | z              |
| multi_control_rotation_gates.py           |                    |             | mcrx           |
|                                           |                    |             | mcry           |
|                                           |                    |             | mcrz           |
| multi_control_toffoli_gate.py             |                    |             | mct            |
| multi_control_u1_gate.py                  |                    |             | mcu1           |
| relative_phase_toffoli.py                 |                    |             | rccx           |
|                                           |                    |             | rcccx          |
| quantum_initializer/diag.py               | DiagGate           | diag        | diag_gate      |
| quantum_initializer/initializer.py        | Initialize         | initialize  | initialize     |
| quantum_initializer/mcg_up_to_diagonal.py | MCGupDiag          | MCGupDiag   |                |
| quantum_initializer/squ.py                | SingleQubitUnitary | unitary     | squ            |
| quantum_initializer/ucg.py                | UCG                | multiplexer | ucg            |
| quantum_initializer/ucrot.py              | UCRot              |             |                |
| quantum_initializer/ucx.py                | UCX(UCRot)         | ucrotX      | ucx            |
| quantum_initializer/ucy.py                | UCY(UCRot)         | ucrotY      | ucy            |
| quantum_initializer/ucz.py                | UCZ(UCRot)         | ucrotZ      | ucz            |
| quantum_initializer/isometry.py           | Isometry           | iso         | iso            |
Cryoris commented 4 years ago

Okay, from that perspective upper case is more sensible. That's fine for me too.

Thanks for the overview. What should we do with the dagger naming? All uppercase (*DG) wouldn't be that sensible since every uppercase letter stands for some concept, so DG would mean two concepts. Both *Dg and *dg would be possible.

chriseclectic commented 4 years ago

I like the idea of all upper case. The identity case is a pet peeve of mine. It's a Pauli so it should be IGate like in other objects that use Pauli labels . However gate is IdGate and circ extension is iden.

I would also like to see a gate namespace that has aliases for the lower case circuit extensions to the gate objects. Eg something like gates.cx => CXGate. Here you could also add additional aliases if desired, eg gates.ccx, gates.toffoli => CCXGate

Also dg for dagger'ed gates might not be obvious to non-maths people what dagger means. It could be replaced with i or inv for inverse. Eg. TiGate or TinvGate.

Cryoris commented 4 years ago

Using inverse instead of dagger'ed is a good idea, I believe. Especially since we're (likely) stepping away from the "math" way of writing Ry in favour RY.

Maybe we should use Inv with a leading capital letter, as all our concepts begin with leading capital letters: Rotations, Controlled, and hence Inverse. (Ruling out I for inverse here, as that collides with a possible naming for the identity Pauli gate.) Also, something like NamedgateInv is probably more readable than Namedgateinv.

Concerning the identity gate, we could rename it to IGate, but would it then also be qc.i in the same style as for all other gates? I think IdGate is the clearer way of naming it, then we can also use it as qc.id. Of course, the current qc.iden could be kept in the aliases (or be deprecated).

ajavadia commented 4 years ago

I would also like to see a gate namespace that has aliases for the lower case circuit extensions to the gate objects. Eg something like gates.cx => CXGate. Here you could also add additional aliases if desired, eg gates.ccx, gates.toffoli => CCXGate

Yes please I have been wanting to move these gates out of qiskit.extensions for 2 years. Moving them to qiskit.circuit.gates sounds good, and exposing a qiskit.gates namespace so that one can do:

from qiskit import gates
circuit.append(gates.cx, [0, 1])

@Cryoris can you start with the gate name standardization/deprecations first and then we do this in steps? Seems everyone is good with uppercase. IGate and qc.i seem fine to me.

ajavadia commented 4 years ago

@kdk there are some gates under qiskit.extensions.quantum_initializer that should be next to where all the other gates are. Like diagonal and uniformly-controlled gates. Can you update your summary above with those?

kdk commented 4 years ago

@kdk there are some gates under qiskit.extensions.quantum_initializer that should be next to where all the other gates are. Like diagonal and uniformly-controlled gates. Can you update your summary above with those?

Updated.

Cryoris commented 4 years ago

@Cryoris can you start with the gate name standardization/deprecations first and then we do this in steps? Seems everyone is good with uppercase. IGate and qc.i seem fine to me.

I opened #3695 as a work-in-progess PR to fix the naming. The names are using the uppercase convention now and the identity gate has been renamed to IGate / qc.i.

I haven't yet changed the name of the dagger'ed/inverted gates, but with the current uppercase naming a suffix inv as proposed by @chriseclectic seems reasonable.

Cryoris commented 4 years ago

Concerning the gates in qiskit/extensions/quantum_initializer:

The uniformly controlled gates in quantum_initializer are currently named UCX/Y/Z for uniformly controlled rotations. They should be named UCRX/Y/Z, since they are rotations. Possibly UCRot should become UCPauliRotation to avoid being associated with the RGate.

The generic uniformly controlled gate is called UCG. Since the letter G is not really associated with an action like controlled or rotation, can we rename this to UCGate to make it immediately obvious what it is? The abbreviations seems a bit cryptic otherwise 🤔

ajavadia commented 4 years ago

I agree with all the above

On Jan 7, 2020, at 9:23 AM, Julien Gacon notifications@github.com wrote:

Concerning the gates in qiskit/extensions/quantum_initializer:

The uniformly controlled gates in quantum_initializer are currently named UCX/Y/Z for uniformly controlled rotations. They should be named UCRX/Y/Z, since they are rotations. Possibly UCRot should become UCPauliRotation to avoid being associated with the RGate.

The generic uniformly controlled gate is called UCG. Since the letter G is not really associated with an action like controlled or rotation, can we rename this to UCGate to make it immediately obvious what it is? The abbreviations seems a bit cryptic otherwise 🤔

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

Cryoris commented 4 years ago

Achieved via #3695 🚀