Rewrite singleton handling including SingletonInstruction (#11014)

* Rewrite singleton handling including `SingletonInstruction`

This is a large rewrite of the singleton gate handling, building off the
work done across the library to make the initial implementation work.

There are two main purposes to this commit:

* Make `SingletonInstruction` available in addition to `SingletonGate`,
  and have these be easy to maintain in conjunction, not need to
  duplicate overrides between them, and not require inheritors to do any
  nasty multiple inheritance or the like.

* Fix regressions in the construction time of `SingletonGate` instances.

In the end, this turned into a fairly complete rewrite that completely
switches out the method of defining the classes; it transpires that the
previous way of having the "immutable overrides" on _all_ instances of
the classes was the main source of slowdown, with `__setattr__` being a
large problem.  The previous method was far easier from an
implementation perspective, but the runtime costs ended up being too
high.

This new form takes a vastly different strategy, explained in detail in
`qiskit/circuit/singleton.py`.  The gist is that we instead make the
singleton instance a _subclass_ of the class object we're creating, and
only it contains the overrides to make itself immutable.  We get around
the instantiation problem (`__init__` isn't special and doesn't
skip the forbidden `__setattr__`) by constructing an instance of the
_base_ class, and then dynamically switching in the overrides
afterwards.  Since the overrides and the dynamic singleton type of the
instance are designed to be co-operative (including in `__slots__`
layout), this can be done safely.

* Make singleton documentation public

* Add tests of new singleton behaviour

* Fix typos

Co-authored-by: Matthew Treinish <[email protected]>

---------

Co-authored-by: Matthew Treinish <[email protected]>

docs/apidoc/circuit_singleton.rst

@@ -0,0 +1,6 @@
+.. _qiskit-circuit-singleton:
+
+.. automodule:: qiskit.circuit.singleton
+   :no-members:
+   :no-inherited-members:
+   :no-special-members:

docs/apidoc/index.rst

@@ -10,6 +10,7 @@ API Reference
    circuit
    circuit_library
    circuit_classical
+   circuit_singleton
    compiler
    execute
    visualization

qiskit/circuit/__init__.py

@@ -279,7 +279,6 @@
    InstructionSet
    Operation
    EquivalenceLibrary
-   SingletonGate
 
 Control Flow Operations
 -----------------------
@@ -359,6 +358,7 @@
 
 .. autoexception:: CircuitError
 """
+
 from .exceptions import CircuitError
 from .quantumcircuit import QuantumCircuit
 from .classicalregister import ClassicalRegister, Clbit
@@ -367,7 +367,7 @@
 
 # pylint: disable=cyclic-import
 from .controlledgate import ControlledGate
-from .singleton_gate import SingletonGate
+from . import singleton
 from .instruction import Instruction
 from .instructionset import InstructionSet
 from .operation import Operation

qiskit/circuit/controlledgate.py

@@ -232,7 +232,10 @@ def params(self, parameters):
             CircuitError: If controlled gate does not define a base gate.
         """
         if self.base_gate:
-            self.base_gate.params = parameters
+            if self.base_gate.mutable:
+                self.base_gate.params = parameters
+            elif parameters:
+                raise CircuitError("cannot set parameters on immutable base gate")
         else:
             raise CircuitError("Controlled gate does not define base gate for extracting params")
 

qiskit/circuit/instruction.py

@@ -31,9 +31,11 @@
 The circuit itself keeps this context.
 """
 
+from __future__ import annotations
+
 import copy
 from itertools import zip_longest
-from typing import List
+from typing import List, Type
 
 import numpy
 
@@ -103,6 +105,38 @@ def __init__(self, name, num_qubits, num_clbits, params, duration=None, unit="dt
 
         self.params = params  # must be at last (other properties may be required for validation)
 
+    @property
+    def base_class(self) -> Type[Instruction]:
+        """Get the base class of this instruction.  This is guaranteed to be in the inheritance tree
+        of ``self``.
+
+        The "base class" of an instruction is the lowest class in its inheritance tree that the
+        object should be considered entirely compatible with for _all_ circuit applications.  This
+        typically means that the subclass is defined purely to offer some sort of programmer
+        convenience over the base class, and the base class is the "true" class for a behavioural
+        perspective.  In particular, you should *not* override :attr:`base_class` if you are
+        defining a custom version of an instruction that will be implemented differently by
+        hardware, such as an alternative measurement strategy, or a version of a parametrised gate
+        with a particular set of parameters for the purposes of distinguishing it in a
+        :class:`.Target` from the full parametrised gate.
+
+        This is often exactly equivalent to ``type(obj)``, except in the case of singleton instances
+        of standard-library instructions.  These singleton instances are special subclasses of their
+        base class, and this property will return that base.  For example::
+
+            >>> isinstance(XGate(), XGate)
+            True
+            >>> type(XGate()) is XGate
+            False
+            >>> XGate().base_class is XGate
+            True
+
+        In general, you should not rely on the precise class of an instruction; within a given
+        circuit, it is expected that :attr:`Instruction.name` should be a more suitable
+        discriminator in most situations.
+        """
+        return type(self)
+
     @property
     def mutable(self) -> bool:
         """Is this instance is a mutable unique instance or not.
@@ -141,8 +175,9 @@ def __eq__(self, other):
         Returns:
             bool: are self and other equal.
         """
-        if (
-            type(self) is not type(other)
+        if (  # pylint: disable=too-many-boolean-expressions
+            not isinstance(other, Instruction)
+            or self.base_class is not other.base_class
             or self.name != other.name
             or self.num_qubits != other.num_qubits
             or self.num_clbits != other.num_clbits
@@ -366,7 +401,13 @@ def reverse_ops(self):
             qiskit.circuit.Instruction: a new instruction with
                 sub-instructions reversed.
         """
-        if not self._definition:
+        # A single `Instruction` cannot really determine whether it is a "composite" instruction or
+        # not; it depends on greater context whether it needs to be decomposed.  The `_definition`
+        # not existing is flaky; all that means is that nobody has _yet_ asked for its definition;
+        # for efficiency, most gates define this on-the-fly.  The checks here are a very very
+        # approximate check for an "atomic" instruction, that are mostly just this way for
+        # historical consistency.
+        if not self._definition or not self.mutable:
             return self.copy()
 
         reverse_inst = self.copy(name=self.name + "_reverse")

qiskit/circuit/library/standard_gates/dcx.py

@@ -12,7 +12,7 @@
 
 """Double-CNOT gate."""
 
-from qiskit.circuit.singleton_gate import SingletonGate
+from qiskit.circuit.singleton import SingletonGate
 from qiskit.circuit.quantumregister import QuantumRegister
 from qiskit.circuit._utils import with_gate_array
 
@@ -48,14 +48,9 @@ class DCXGate(SingletonGate):
             \end{pmatrix}
     """
 
-    def __init__(self, label=None, duration=None, unit=None, _condition=None):
+    def __init__(self, label=None, *, duration=None, unit="dt"):
         """Create new DCX gate."""
-        if unit is None:
-            unit = "dt"
-
-        super().__init__(
-            "dcx", 2, [], label=label, _condition=_condition, duration=duration, unit=unit
-        )
+        super().__init__("dcx", 2, [], label=label, duration=duration, unit=unit)
 
     def _define(self):
         """

qiskit/circuit/library/standard_gates/ecr.py

@@ -16,7 +16,7 @@
 
 from qiskit.circuit._utils import with_gate_array
 from qiskit.circuit.quantumregister import QuantumRegister
-from qiskit.circuit.singleton_gate import SingletonGate
+from qiskit.circuit.singleton import SingletonGate
 from .rzx import RZXGate
 from .x import XGate
 
@@ -84,13 +84,9 @@ class ECRGate(SingletonGate):
                 \end{pmatrix}
     """
 
-    def __init__(self, label=None, _condition=None, duration=None, unit=None):
+    def __init__(self, label=None, *, duration=None, unit="dt"):
         """Create new ECR gate."""
-        if unit is None:
-            unit = "dt"
-        super().__init__(
-            "ecr", 2, [], label=label, _condition=_condition, duration=duration, unit=unit
-        )
+        super().__init__("ecr", 2, [], label=label, duration=duration, unit=unit)
 
     def _define(self):
         """

qiskit/circuit/library/standard_gates/h.py

@@ -15,7 +15,7 @@
 from typing import Optional, Union
 import numpy
 from qiskit.circuit.controlledgate import ControlledGate
-from qiskit.circuit.singleton_gate import SingletonGate
+from qiskit.circuit.singleton import SingletonGate
 from qiskit.circuit.quantumregister import QuantumRegister
 from qiskit.circuit._utils import with_gate_array, with_controlled_gate_array
 from .t import TGate, TdgGate
@@ -54,13 +54,9 @@ class HGate(SingletonGate):
             \end{pmatrix}
     """
 
-    def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
+    def __init__(self, label: Optional[str] = None, *, duration=None, unit="dt"):
         """Create new H gate."""
-        if unit is None:
-            unit = "dt"
-        super().__init__(
-            "h", 1, [], label=label, _condition=_condition, duration=duration, unit=unit
-        )
+        super().__init__("h", 1, [], label=label, duration=duration, unit=unit)
 
     def _define(self):
         """

qiskit/circuit/library/standard_gates/i.py

@@ -13,7 +13,7 @@
 """Identity gate."""
 
 from typing import Optional
-from qiskit.circuit.singleton_gate import SingletonGate
+from qiskit.circuit.singleton import SingletonGate
 from qiskit.circuit._utils import with_gate_array
 
 
@@ -45,13 +45,9 @@ class IGate(SingletonGate):
              └───┘
     """
 
-    def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
+    def __init__(self, label: Optional[str] = None, *, duration=None, unit="dt"):
         """Create new Identity gate."""
-        if unit is None:
-            unit = "dt"
-        super().__init__(
-            "id", 1, [], label=label, _condition=_condition, duration=duration, unit=unit
-        )
+        super().__init__("id", 1, [], label=label, duration=duration, unit=unit)
 
     def inverse(self):
         """Invert this gate."""

qiskit/circuit/library/standard_gates/iswap.py

@@ -16,7 +16,7 @@
 
 import numpy as np
 
-from qiskit.circuit.singleton_gate import SingletonGate
+from qiskit.circuit.singleton import SingletonGate
 from qiskit.circuit.quantumregister import QuantumRegister
 from qiskit.circuit._utils import with_gate_array
 
@@ -85,13 +85,9 @@ class iSwapGate(SingletonGate):
             \end{pmatrix}
     """
 
-    def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
+    def __init__(self, label: Optional[str] = None, *, duration=None, unit="dt"):
         """Create new iSwap gate."""
-        if unit is None:
-            unit = "dt"
-        super().__init__(
-            "iswap", 2, [], label=label, _condition=_condition, duration=duration, unit=unit
-        )
+        super().__init__("iswap", 2, [], label=label, duration=duration, unit=unit)
 
     def _define(self):
         """

qiskit/circuit/library/standard_gates/s.py

@@ -18,7 +18,7 @@
 import numpy
 
 from qiskit.circuit.controlledgate import ControlledGate
-from qiskit.circuit.singleton_gate import SingletonGate
+from qiskit.circuit.singleton import SingletonGate
 from qiskit.circuit.library.standard_gates.p import CPhaseGate, PhaseGate
 from qiskit.circuit.quantumregister import QuantumRegister
 from qiskit.circuit._utils import with_gate_array, with_controlled_gate_array
@@ -59,13 +59,9 @@ class SGate(SingletonGate):
     Equivalent to a :math:`\pi/2` radian rotation about the Z axis.
     """
 
-    def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
+    def __init__(self, label: Optional[str] = None, *, duration=None, unit="dt"):
         """Create new S gate."""
-        if unit is None:
-            unit = "dt"
-        super().__init__(
-            "s", 1, [], label=label, _condition=_condition, duration=duration, unit=unit
-        )
+        super().__init__("s", 1, [], label=label, duration=None, unit="dt")
 
     def _define(self):
         """
@@ -124,13 +120,9 @@ class SdgGate(SingletonGate):
     Equivalent to a :math:`-\pi/2` radian rotation about the Z axis.
     """
 
-    def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
+    def __init__(self, label: Optional[str] = None, *, duration=None, unit="dt"):
         """Create new Sdg gate."""
-        if unit is None:
-            unit = "dt"
-        super().__init__(
-            "sdg", 1, [], label=label, _condition=_condition, duration=duration, unit=unit
-        )
+        super().__init__("sdg", 1, [], label=label, duration=None, unit="dt")
 
     def _define(self):
         """

qiskit/circuit/library/standard_gates/swap.py

@@ -15,7 +15,7 @@
 from typing import Optional, Union
 import numpy
 from qiskit.circuit.controlledgate import ControlledGate
-from qiskit.circuit.singleton_gate import SingletonGate
+from qiskit.circuit.singleton import SingletonGate
 from qiskit.circuit.quantumregister import QuantumRegister
 from qiskit.circuit._utils import with_gate_array, with_controlled_gate_array
 
@@ -59,13 +59,9 @@ class SwapGate(SingletonGate):
         |a, b\rangle \rightarrow |b, a\rangle
     """
 
-    def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
+    def __init__(self, label: Optional[str] = None, *, duration=None, unit="dt"):
         """Create new SWAP gate."""
-        if unit is None:
-            unit = "dt"
-        super().__init__(
-            "swap", 2, [], label=label, _condition=_condition, duration=duration, unit=unit
-        )
+        super().__init__("swap", 2, [], label=label, duration=duration, unit=unit)
 
     def _define(self):
         """

qiskit/circuit/library/standard_gates/sx.py

@@ -15,7 +15,7 @@
 from math import pi
 from typing import Optional, Union
 from qiskit.circuit.controlledgate import ControlledGate
-from qiskit.circuit.singleton_gate import SingletonGate
+from qiskit.circuit.singleton import SingletonGate
 from qiskit.circuit.quantumregister import QuantumRegister
 from qiskit.circuit._utils import with_gate_array, with_controlled_gate_array
 
@@ -63,13 +63,9 @@ class SXGate(SingletonGate):
 
     """
 
-    def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
+    def __init__(self, label: Optional[str] = None, *, duration=None, unit="dt"):
         """Create new SX gate."""
-        if unit is None:
-            unit = "dt"
-        super().__init__(
-            "sx", 1, [], label=label, _condition=_condition, duration=duration, unit=unit
-        )
+        super().__init__("sx", 1, [], label=label, duration=duration, unit=unit)
 
     def _define(self):
         """
@@ -146,13 +142,9 @@ class SXdgGate(SingletonGate):
 
     """
 
-    def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
+    def __init__(self, label: Optional[str] = None, *, duration=None, unit="dt"):
         """Create new SXdg gate."""
-        if unit is None:
-            unit = "dt"
-        super().__init__(
-            "sxdg", 1, [], label=label, _condition=_condition, duration=duration, unit=unit
-        )
+        super().__init__("sxdg", 1, [], label=label, duration=duration, unit=unit)
 
     def _define(self):
         """