Consistent naming of the standard gates by Cryoris · Pull Request #3695 · Qiskit/qiskit

Cryoris · 2020-01-07T13:16:05Z

Summary

Ensure consistent naming of the standard gates according to #3633

Details and comments

This PR ensures that the names of the standard gates follow the same consistent principles. See #3633 for more detail and the discussion on gate naming. The old/current gates will be deprecated.

Up-to-date list of the names:

| file                            | class                    | name           | qc._           |
|---------------------------------+--------------------------+----------------+----------------|
  extensions/standard                 
| barrier.py                      | Barrier                  | barrier        | barrier        |
| ccx.py                          | CCXGate <- ToffoliGate   | ccx            | ccx, toffoli   |
| ch.py                           | CHGate                   | ch             | ch             |
| crx.py                          | CRXGate <- CrxGate       | crx            | crx            |
| cry.py                          | CRYGate <- CryGate       | cry            | cry            |
| crz.py                          | CRZGate <- CrzGate       | crz            | crz            |
| cswap.py                        | CSwapGate <- FredkinGate | cswap          | cswap, fredkin |
| cu1.py                          | CU1Gate <- Cu1Gate       | cu1            | cu1            |
| cu3.py                          | CU3Gate <- Cu3Gate       | cu3            | cu3            |
| cx.py                           | CXGate <- CnotGate       | cx             | cx, cnot       |
| cy.py                           | CYGate <- CyGate         | cy             | cy             |
| cz.py                           | CZGate <- CzGate         | cz             | cz             |
| h.py                            | HGate                    | h              | h              |
| iden.py                         | IGate <- IdGate          | id             | i/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          |
   extensions/quantum_initializer
| diagonal.py <- diag.py          | DiagonalGate <- DiagGate | diagonal <- diag | diagonal <- diag_gate
| initializer.py                  | Initialize               | initialize     | initialize     |
| mcg_up_to_diagonal.py           | MCGupDiag                | MCGupDiag      |                |
| squ.py                          | SingleQubitUnitary       | unitary        | squ            |
| uc.py <- ucg.py                 | UCGate <- UCG            | multiplexer    | uc <- ucg      |
| uc_pauli_rot.py <- ucrot.py     | UCPauliRotGate <- UCRot  |                |                |
| ucrx.py <- ucx.py               | UCRXGate <- UCX          | ucrx <- ucrotX | ucrx <- ucx    |
| ucry.py <- ucy.py               | UCRYGate <- UCY          | ucry <- ucrotY | ucry <- ucy    |
| ucrz.py <- ucz.py               | UCRZGate <- UCZ          | ucrz <- ucrotZ | ucrz <- ucz    |
| isometry.py                     | Isometry                 | iso            | iso            |

Gate checklist:

Note: MCGupDiag is left as is, this refactor will be tackled via #3761.

Cryoris · 2020-01-07T13:29:56Z

For the deprecation warnings we could have two mechanisms (and probably also others).
The old gate names could be functions returning the new class:

def ToffoliGate():
    import warnings
    warnings.warn('ToffoliGate is deprecated, use CCXGate instead!', DeprecationWarning, 2)
    return CCXGate()

or we could derive the old class from the new one, to still give access to something of the old-class type:

class ToffoliGate(CCXGate):
    def __init__(self):
        import warnings
        warnings.warn('ToffoliGate is deprecated, use CCXGate instead!', DeprecationWarning, 2)
        super().__init__()

Latter seems safer to me, since we still have the old class available as type.

Is there another standard procedure to deprecate class names?

kdk · 2020-01-07T14:24:21Z

The overall approach looks good. For the deprecation, we should think through what cases we want to cover, because I agree handling it in full generality wouldn't be straightforward. While the second example supports isinstance(ToffoliGate(), CCXGate), it doesn't support isinstance(CCXGate(), ToffoliGate).

It might be beneficial to split this in two PRs. One to rename and deprecate the to-be-renamed classes, and another to update all the instances where they are used.

Cryoris · 2020-01-07T14:28:22Z

While the second example supports isinstance(ToffoliGate(), CCXGate), it doesn't support isinstance(CCXGate(), ToffoliGate).

That's correct, but this instance-checking is not really something we expect anybody to do, or is it?

ewinston · 2020-01-07T23:00:59Z

To pass @kdk's check you may be able to define a metaclass like

class ToffoliGateMeta(type):
    def __instancecheck__(self, inst):
        if type(inst) in {CCXGate, ToffoliGate}:
            return True
        return False

class CCXGate(ControlledGate, metaclass=ToffoliGateMeta):

...

class ToffoliGate(CCXGate, metaclass=ToffoliGateMeta):
    def __init__(self):
        import warnings
        warnings.warn('ToffoliGate is deprecated, use CCXGate instead!', DeprecationWarning, 2)
        super().__init__()

Cryoris · 2020-01-08T13:50:41Z

Nice workaround (though it seems like a lot of work to deprecate a class). But to ensure everything's running smoothly we probably should implement that.

Edit: @ewinston's proposal to pass @kdk's check has been implemented.

ewinston

Overall looks good. Some comments I have;

Since this PR has many user facing changes could you add deprecations to the release notes. The procedure for this is described in "CONTRIBUTING.md".
In some gate classes you prepended "The" to the docstring. I think I prefer without but it's probably more in line with PEP8 to have a complete sentence. It just could be more consistent.

Cryoris · 2020-01-09T08:10:12Z

Edit: The up-to-date list is in the description of this PR.

Updating @kdk's list from #3633, the gate names are now

| file                            | class                    | name           | qc._           |
|---------------------------------+--------------------------+----------------+----------------|
  extensions/standard                 
| barrier.py                      | Barrier                  | barrier        | barrier        |
| ccx.py                          | CCXGate <- ToffoliGate   | ccx            | ccx, toffoli   |
| ch.py                           | CHGate                   | ch             | ch             |
| crx.py                          | CRXGate <- CrxGate       | crx            | crx            |
| cry.py                          | CRYGate <- CryGate       | cry            | cry            |
| crz.py                          | CRZGate <- CrzGate       | crz            | crz            |
| cswap.py                        | CSwapGate <- FredkinGate | cswap          | cswap, fredkin |
| cu1.py                          | CU1Gate <- Cu1Gate       | cu1            | cu1            |
| cu3.py                          | CU3Gate <- Cu3Gate       | cu3            | cu3            |
| cx.py                           | CXGate <- CnotGate       | cx             | cx, cnot       |
| cy.py                           | CYGate <- CyGate         | cy             | cy             |
| cz.py                           | CZGate <- CzGate         | cz             | cz             |
| h.py                            | HGate                    | h              | h              |
| iden.py                         | IGate <- IdGate          | id             | i/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          |
   extensions/quantum_initializer
| diag.py                         | DiagGate                 | diag           | diag_gate      |
| initializer.py                  | Initialize               | initialize     | initialize     |
| mcg_up_to_diagonal.py           | MCGupDiag                | MCGupDiag      |                |
| squ.py                          | SingleQubitUnitary       | unitary        | squ            |
| uc.py <- ucg.py                 | UCGate <- UCG            | multiplexer    | uc <- ucg      |
| uc_pauli_rot.py <- ucrot.py     | UCPauliRotGate <- UCRot  |                |                |
| ucrx.py <- ucx.py               | UCRXGate <- UCX          | ucrx <- ucrotX | ucrx <- ucx    |
| ucry.py <- ucy.py               | UCRYGate <- UCY          | ucry <- ucrotY | ucry <- ucy    |
| ucrz.py <- ucz.py               | UCRZGate <- UCZ          | ucrz <- ucrotZ | ucrz <- ucz    |
| isometry.py                     | Isometry                 | iso            | iso            |

This reverts commit cd13878.

This reverts commit f1be570.

merge conflict resolution for "consistent gate naming"

Cryoris · 2020-02-19T18:01:41Z

I noticed that the gates in quantum_initializer have been renamed without keeping the old class for deprecation purposes. Afaik these gates are only used for the isometry though. Should I add deprecations or is that fine?

ajavadia · 2020-02-19T18:30:47Z

@Cryoris I think I've seen diagonal gate being used by people, so yeah it would be good to deprecate those too.

Cryoris · 2020-02-19T23:14:31Z

@ajavadia I added the previous classes with a deprecation warning. Also I added a test to

instantiating all renamed old and new classes
check the old class is a subclass of the new one (to have the same behaviour)
check that both the old and new class are instances of one another (Kevin's comment)

ajavadia · 2020-02-21T21:32:35Z

Thanks for this effort!

* rename the gates according to terra issue Qiskit#3633 * fix typos, quotes * replace gate names in qiskit and test dirs * replace qc.iden by qc.i * fix lint, and some leftover renamings * add matrix representation of CY * UCG -> UCGate and .ucg -> uc * UC(X,Y,Z) -> UCR(X,Y,Z)Gate and uc(x,y,z) -> ucr(x,y,z) * fix line length * fix lint * add deprecation warnings for old classes * fix missing theta arg * fix test, test type via subclass not "== type" * add reno changelog for the gate names * rename UCRot -> UCPauliRotGate * rename ucrot file, fix line len * rename Sdg -> Sinv, sdg -> sinv * fix metaclass instancecheck using isinstance() as required by pylint caused unterminated recursive call of instancecheck * fix parser test sinv is one char longer than sdg, thus the parsed commands are longer (3 appearances, therefore 3 chars longer) * rename Tdg -> Tinv, tdg -> tinv * fix text drawer test * update to non-deprecated qc methods * test.python.test_qasm_parser.TestParser.test_parser * support both *dg/*inv in qelib1.inc * *dg must remain standard extensions in AstInterpreter * name: ucrot(X,Y,Z) -> ucr(x,y,z), fix (doc)strings along the way * add test on all qelib1.inc gates * fix *inv -> *Inv naming * fix missing renamings, use all_gates.qasm instead of a str * correct gate label for sinv/tinv * start renaming "id" -> "i" * rename all id -> i * reno: fix typos, update gate list * add i gate to qasm * deprecation warnings adhere to contribution guidelines * fix id json keyword * fix modified png file * remove obsolete if case * set deprecation version to 0.12.0 * revert changes on backend configs * keep sdg,tdg,id as valid aliases * revert names to id/sdg/tdg * update error message error message on "i" changed back to "id" * avoid cyclic imports * rename the method diag_gate to diag, update reno * add comment why Initialize is not unitary * change deprecation version to 0.14.0 * fix lints, rename diag -> diagonal * fix deprecation on import * fix Cnot/CX naming * fix deprecation version * fix return of multiplexer pauli rotations * name swap uppercase throughout * rename iso -> isometry (keep alias) * Update deprecation warning Co-Authored-By: Luciano Bello <luciano.bello@ibm.com> * Update deprecation warning of ucx Co-Authored-By: Luciano Bello <luciano.bello@ibm.com> * Update deprecation warning of ucz Co-Authored-By: Luciano Bello <luciano.bello@ibm.com> * Update deprecation warning of ucy Co-Authored-By: Luciano Bello <luciano.bello@ibm.com> * fix redefine outer scope * attempt fixing jupyter cells * move matrix repr from Fredkin to CSwap * add reno * Revert "move matrix repr from Fredkin to CSwap" This reverts commit cd13878. * Revert "add reno" This reverts commit f1be570. * unrevert revert * fix leftover Cnot * keep old quantum initializer classes but deprecate * fix lint & test * add test for the deprecated classes Co-authored-by: Luciano Bello <luciano.bello@ibm.com> Co-authored-by: ewinston <ewinston@us.ibm.com> Co-authored-by: Ali Javadi-Abhari <ajavadia@users.noreply.github.com>

Cryoris added 5 commits January 7, 2020 13:01

rename the gates according to terra issue Qiskit#3633

facebce

fix typos, quotes

90a3f77

replace gate names in qiskit and test dirs

88ffc11

replace qc.iden by qc.i

9321041

fix lint, and some leftover renamings

6ba88cb

Cryoris requested review from 1ucian0, ajavadia, chriseclectic, ewinston, kdk and mtreinish as code owners January 7, 2020 13:16

Cryoris requested a review from nonhermitian as a code owner January 7, 2020 13:45

Cryoris mentioned this pull request Jan 7, 2020

Consistent naming of controlled gates #3633

Closed

Cryoris added 4 commits January 7, 2020 15:42

add matrix representation of CY

088fcb5

UCG -> UCGate and .ucg -> uc

b874a38

UC(X,Y,Z) -> UCR(X,Y,Z)Gate and uc(x,y,z) -> ucr(x,y,z)

a6aeb19

fix line length

9630d37

fix lint

802500f

Cryoris added 4 commits January 8, 2020 17:35

add deprecation warnings for old classes

cb57b88

fix missing theta arg

75a6528

Merge branch 'master' into consistent_gate_naming

5070665

fix test, test type via subclass not "== type"

2b0642c

kdk assigned ewinston Jan 8, 2020

ewinston suggested changes Jan 8, 2020

View reviewed changes

Cryoris and others added 11 commits February 14, 2020 09:36

fix redefine outer scope

de3588b

attempt fixing jupyter cells

4fb3ca8

move matrix repr from Fredkin to CSwap

cd13878

add reno

f1be570

Revert "move matrix repr from Fredkin to CSwap"

b05a9d1

This reverts commit cd13878.

Revert "add reno"

3e0fb78

This reverts commit f1be570.

unrevert revert

e09731c

Merge branch 'master' into consistent_gate_naming

95229ab

Merge branch 'master' into Cryoris-consistent_gate_naming

b6b94ac

Merge pull request #2 from ewinston/Cryoris-consistent_gate_naming

1f0f597

merge conflict resolution for "consistent gate naming"

fix leftover Cnot

341b801

ewinston previously approved these changes Feb 19, 2020

View reviewed changes

Cryoris added 2 commits February 19, 2020 21:11

keep old quantum initializer classes but deprecate

8e82d5c

fix lint & test

780f5e9

Cryoris dismissed ewinston’s stale review via 780f5e9 February 19, 2020 22:42

add test for the deprecated classes

e5d9d15

Merge branch 'master' into consistent_gate_naming

cbf47c7

ewinston approved these changes Feb 21, 2020

View reviewed changes

ajavadia approved these changes Feb 21, 2020

View reviewed changes

ajavadia added the automerge label Feb 21, 2020

Merge branch 'master' into consistent_gate_naming

c6cadf7

mergify Bot merged commit f1814d7 into Qiskit:master Feb 21, 2020

ajavadia mentioned this pull request Mar 19, 2020

text drawer should capitalize gate names #3990

Closed

Cryoris deleted the consistent_gate_naming branch January 11, 2021 20:41

Cryoris mentioned this pull request Mar 17, 2025

Concise StandardGate::<ID> enums in Rust space #14026

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consistent naming of the standard gates#3695

Consistent naming of the standard gates#3695
mergify[bot] merged 85 commits into
Qiskit:masterfrom
Cryoris:consistent_gate_naming

Cryoris commented Jan 7, 2020 •

edited

Loading

Uh oh!

Cryoris commented Jan 7, 2020

Uh oh!

kdk commented Jan 7, 2020

Uh oh!

Cryoris commented Jan 7, 2020

Uh oh!

ewinston commented Jan 7, 2020

Uh oh!

Cryoris commented Jan 8, 2020 •

edited

Loading

Uh oh!

ewinston left a comment

Uh oh!

Cryoris commented Jan 9, 2020 •

edited

Loading

Uh oh!

Cryoris commented Feb 19, 2020

Uh oh!

ajavadia commented Feb 19, 2020

Uh oh!

Cryoris commented Feb 19, 2020

Uh oh!

ajavadia commented Feb 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Cryoris commented Jan 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details and comments

Uh oh!

Cryoris commented Jan 7, 2020

Uh oh!

kdk commented Jan 7, 2020

Uh oh!

Cryoris commented Jan 7, 2020

Uh oh!

ewinston commented Jan 7, 2020

Uh oh!

Cryoris commented Jan 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ewinston left a comment

Choose a reason for hiding this comment

Uh oh!

Cryoris commented Jan 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Cryoris commented Feb 19, 2020

Uh oh!

ajavadia commented Feb 19, 2020

Uh oh!

Cryoris commented Feb 19, 2020

Uh oh!

ajavadia commented Feb 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Cryoris commented Jan 7, 2020 •

edited

Loading

Cryoris commented Jan 8, 2020 •

edited

Loading

Cryoris commented Jan 9, 2020 •

edited

Loading