Skip to content

Commit

Permalink
Use singletons for standard library unparameterized, non-controlled g…
Browse files Browse the repository at this point in the history
…ates (Qiskit#10314)

* Use singletons for standard library unparameterized, non-controlled gates

This commit adds a new class SingletonGate which is a Gate subclass that
reuses a single instance by default for all instances of a particular
class. This greatly reduces the memory overhead and significant improves
the construction speed for making multiple instances of the same gate.
The tradeoff is in the flexibility of use because it precludes having
any potentially mutable state in the shared instance. This is a large
change to the data model of qiskit because it previously could be
assumed that any gate instance was unique and there weren't any
unintended side effects from modifying it in isolation (for example
doing XGate().label = 'foo' wouldn't potentially break other code).
To limit the impact around this instances of SingletonGate do not allow
mutation of an existing instance. This can (and likely will) cause
unexpected issues as usage of the class is released. Specifically what
used to be valid will now raise an exception because it is a shared
instance. This is evident from the code modifications necessary to
most of the Qiskit code base to enable working with instances of
SingletonGates. The knock on effects of this downstream are likely
significant and managing how we roll this feature out is going to be
equally if not more important than the feature itself. This is why
I'm not personally convinced we want to do all this commit includes
in a single release. I've opened this as a pull request primarily to
start the conversation on how we want to do the roll out to try and
minimize and notify downstream users of the potential breakage to
avoid issues. The primary issue I have is this doesn't really follow
the Qiskit deprecation policy as there is no user facing notification
or documentation of this pending change and code that worked in the
previously release will not work in the release with this feature.
For some aspects of this change (primarily the setters on gate
attributes) this can easily be handled by deprecating it in planned
singleton standard library gates and waiting the necessary amount of
time. But the more fundamental data model changes are hard to announce
ahead of time. We can have a release note about it coming in the future
but it will end up being very abstract and users will not necessarily
be able to act on it ahead of time without concrete examples to test
with. This was an issue for me in developing this feature as I couldn't
anticipate where API breakages would occur until I switched over all the
standard library gates, and there still might be others.

Due to the large performance gains this offers and also in the
interest of testing the API implications of using singleton gates the
unparameterized and non-controlled gates available in
qiskit.circuit.library.standard_gates are all updated to be subclasses
of singleton gates. In aggregate this is causing construction to be
roughly 6x faster and building circuits comprised solely of these gates
consume 1/4th the memory as before. But it also exposed a large number
of internal changes we needed to make to the wider circuit, QPY, qasm2,
dagcircuit, transpiler, converters, and test modules to support working
with singleton gates.

Besides this there are a couple seemingly unrelated API changes in
this PR and it is caused by inconsistencies in the Instruction/Gate
API that were preventing this from working. The first which is the
ECRGate class was missing a label kwarg in the parent. Similarly
all Gate classes and subclasses were missing duration and unit
kwargs on their constructors. These are necessary to be able to use
these fields on singletons because we need an interface to construct
an instance that has the state set so we avoid the use of the global
shared instance. In the release notes I labeled these as bugfixes,
because in all the cases the parent clases were exposing these
interfaces and it primarily is an oversight that they were missing
in these places. But personally this does seem more like something
we'd normally document as a feature rather than a bugfix.

A follow up PR will add a SingletonControlledGate class which will
be similar to SingletonGate but will offer two singleton instance
based on the value of ctrl_state (and also handle nested labels
and other nested mutable state in the base gate). We can then update
the standard library gates like CXGate, and CHGate to also be
singletons. The ctrl state attribute is primarily why these gates
were not included in this commit.

* Fix Python 3.8 compatibility

There are some differences in how the inspect stdlib module behaves
between python 3.8 and newer versions of python. This was causing
divergence in the test and qpy behavior where inspect was used to
determine different aspects of a gate (either whether label was
supported as an arg or find the number of free parameters). This commit
fixes this by adjusting the code to handle both newer versions of
inspect as well as older ones.

* Simplify qpy deserialization label handling

* Remove unused classmethod decorator

* Fix lint

* Fix timeline drawer on output of legacy DD pass

* Fix doc build

* Add methods to deal with mutability of singleton gates

This commit adds two methods to the SingletonGate class, mutable and
to_mutable. The mutable() method is a property method that returns
whether a particular instance is mutable or a shared singleton instance.
The second method to_mutable() returns a mutable copy of the gate.

* Disallow custom attribute on singleton instances

This commit adds a __setattr__ method to the singleton gate class to
ensure that custom attributes are not settable for a shared singleton
instance. It prevents addign a custom attribute if the instance is in
singleton mode and will raise a NotImplementedError to avoid silently
sharing state in the single shared instance.

* Fix rebase error

* Fix rebase issues

* Fix module docstring

* Add .mutable and .to_mutable() to Instruction

To unify the access patterns between SingletonGates and other
instructions this commit adds a common property mutable and method
to_mutable() to check if any Instruction (not just singletons) are
mutable or not and to get a mutable copy. For things that don't inherit
from SingletonGate these are hard coded to `True` and to return a copy
as by default every Instruction is mutable, only `SingletonGate` objects
are different in this regard.

* Unify handling of gates in scheduling passes

Previously we were explicitly handling the SingletonGate class in the
scheduling passes. But with the introduction of mutable and to_mutable()
on Instruction we don't need to condition on gates being singleton or
not and we can just handle them in the same manner as other
instructions. This commit implements this change.

* Remove unnecessary deepcopy in substitute_node_wtih_dag

* Fix logic for SingletonGate.copy

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

* Update Singleton Gate class docstring

* Remove unused imports

* Update release notes

* Fix release note typos

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Improve setattr performance

* Fix deepcopy logic

* Add check for kwargs up front

* Fix docs typos

* Add comment on to_mutable __init__ call

* Fix lint

* Handle positional initialization arguments in SingletonGate

If there are any positional arguments set when initializing a new
SingletonGate subclass those were not being handled correctly before. If
there is a positional argument being set that would indicate at least a
label is being set and indicates a mutable instance should be returned
instead of the immutable singleton. This commit adds the missing check
to the __new__ logic and also adds a test to verify the behavior is
correct.

---------

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: Jake Lishman <jake@binhbar.com>
  • Loading branch information
3 people committed Sep 19, 2023
1 parent 2d3632f commit e62c86b
Show file tree
Hide file tree
Showing 36 changed files with 950 additions and 169 deletions.
2 changes: 2 additions & 0 deletions qiskit/circuit/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@
InstructionSet
Operation
EquivalenceLibrary
SingletonGate
Control Flow Operations
-----------------------
Expand Down Expand Up @@ -366,6 +367,7 @@

# pylint: disable=cyclic-import
from .controlledgate import ControlledGate
from .singleton_gate import SingletonGate
from .instruction import Instruction
from .instructionset import InstructionSet
from .operation import Operation
Expand Down
4 changes: 3 additions & 1 deletion qiskit/circuit/add_control.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ def add_control(
# attempt decomposition
operation._define()
cgate = control(operation, num_ctrl_qubits=num_ctrl_qubits, label=label, ctrl_state=ctrl_state)
cgate.base_gate.label = operation.label
if operation.label is not None:
cgate.base_gate = cgate.base_gate.to_mutable()
cgate.base_gate.label = operation.label
return cgate


Expand Down
12 changes: 10 additions & 2 deletions qiskit/circuit/gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@
class Gate(Instruction):
"""Unitary gate."""

def __init__(self, name: str, num_qubits: int, params: list, label: str | None = None) -> None:
def __init__(
self,
name: str,
num_qubits: int,
params: list,
label: str | None = None,
duration=None,
unit="dt",
) -> None:
"""Create a new gate.
Args:
Expand All @@ -34,7 +42,7 @@ def __init__(self, name: str, num_qubits: int, params: list, label: str | None =
label: An optional label for the gate.
"""
self.definition = None
super().__init__(name, num_qubits, 0, params, label=label)
super().__init__(name, num_qubits, 0, params, label=label, duration=duration, unit=unit)

# Set higher priority than Numpy array and matrix classes
__array_priority__ = 20
Expand Down
33 changes: 30 additions & 3 deletions qiskit/circuit/instruction.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,43 @@ def __init__(self, name, num_qubits, num_clbits, params, duration=None, unit="dt
self._label = label
# tuple (ClassicalRegister, int), tuple (Clbit, bool) or tuple (Clbit, int)
# when the instruction has a conditional ("if")
self.condition = None
self._condition = None
# list of instructions (and their contexts) that this instruction is composed of
# empty definition means opaque or fundamental instruction
self._definition = None

self._duration = duration
self._unit = unit

self.params = params # must be at last (other properties may be required for validation)

@property
def mutable(self) -> bool:
"""Is this instance is a mutable unique instance or not.
If this attribute is ``False`` the gate instance is a shared singleton
and is not mutable.
"""
return True

def to_mutable(self):
"""Return a mutable copy of this gate.
This method will return a new mutable copy of this gate instance.
If a singleton instance is being used this will be a new unique
instance that can be mutated. If the instance is already mutable it
will be a deepcopy of that instance.
"""
return self.copy()

@property
def condition(self):
"""The classical condition on the instruction."""
return self._condition

@condition.setter
def condition(self, condition):
self._condition = condition

def __eq__(self, other):
"""Two instructions are the same if they have the same name,
same dimensions, and same params.
Expand Down Expand Up @@ -409,7 +436,7 @@ def c_if(self, classical, val):
# Casting the conditional value as Boolean when
# the classical condition is on a classical bit.
val = bool(val)
self.condition = (classical, val)
self._condition = (classical, val)
return self

def copy(self, name=None):
Expand Down
2 changes: 1 addition & 1 deletion qiskit/circuit/instructionset.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def c_if(self, classical: Clbit | ClassicalRegister | int, val: int) -> "Instruc
if self._requester is not None:
classical = self._requester(classical)
for instruction in self._instructions:
instruction.operation.c_if(classical, val)
instruction.operation = instruction.operation.c_if(classical, val)
return self

# Legacy support for properties. Added in Terra 0.21 to support the internal switch in
Expand Down
13 changes: 9 additions & 4 deletions qiskit/circuit/library/standard_gates/dcx.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@

"""Double-CNOT gate."""

from qiskit.circuit.gate import Gate
from qiskit.circuit.singleton_gate import SingletonGate
from qiskit.circuit.quantumregister import QuantumRegister
from qiskit.circuit._utils import with_gate_array


@with_gate_array([[1, 0, 0, 0], [0, 0, 0, 1], [0, 1, 0, 0], [0, 0, 1, 0]])
class DCXGate(Gate):
class DCXGate(SingletonGate):
r"""Double-CNOT gate.
A 2-qubit Clifford gate consisting of two back-to-back
Expand Down Expand Up @@ -48,9 +48,14 @@ class DCXGate(Gate):
\end{pmatrix}
"""

def __init__(self):
def __init__(self, label=None, duration=None, unit=None, _condition=None):
"""Create new DCX gate."""
super().__init__("dcx", 2, [])
if unit is None:
unit = "dt"

super().__init__(
"dcx", 2, [], label=label, _condition=_condition, duration=duration, unit=unit
)

def _define(self):
"""
Expand Down
12 changes: 8 additions & 4 deletions qiskit/circuit/library/standard_gates/ecr.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@
import numpy as np

from qiskit.circuit._utils import with_gate_array
from qiskit.circuit.gate import Gate
from qiskit.circuit.quantumregister import QuantumRegister
from qiskit.circuit.singleton_gate import SingletonGate
from .rzx import RZXGate
from .x import XGate


@with_gate_array(
sqrt(0.5) * np.array([[0, 1, 0, 1.0j], [1, 0, -1.0j, 0], [0, 1.0j, 0, 1], [-1.0j, 0, 1, 0]])
)
class ECRGate(Gate):
class ECRGate(SingletonGate):
r"""An echoed cross-resonance gate.
This gate is maximally entangling and is equivalent to a CNOT up to
Expand Down Expand Up @@ -84,9 +84,13 @@ class ECRGate(Gate):
\end{pmatrix}
"""

def __init__(self):
def __init__(self, label=None, _condition=None, duration=None, unit=None):
"""Create new ECR gate."""
super().__init__("ecr", 2, [])
if unit is None:
unit = "dt"
super().__init__(
"ecr", 2, [], label=label, _condition=_condition, duration=duration, unit=unit
)

def _define(self):
"""
Expand Down
30 changes: 22 additions & 8 deletions qiskit/circuit/library/standard_gates/h.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from typing import Optional, Union
import numpy
from qiskit.circuit.controlledgate import ControlledGate
from qiskit.circuit.gate import Gate
from qiskit.circuit.singleton_gate 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
Expand All @@ -25,7 +25,7 @@


@with_gate_array(_H_ARRAY)
class HGate(Gate):
class HGate(SingletonGate):
r"""Single-qubit Hadamard gate.
This gate is a \pi rotation about the X+Z axis, and has the effect of
Expand Down Expand Up @@ -54,9 +54,13 @@ class HGate(Gate):
\end{pmatrix}
"""

def __init__(self, label: Optional[str] = None):
def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
"""Create new H gate."""
super().__init__("h", 1, [], label=label)
if unit is None:
unit = "dt"
super().__init__(
"h", 1, [], label=label, _condition=_condition, duration=duration, unit=unit
)

def _define(self):
"""
Expand Down Expand Up @@ -94,8 +98,7 @@ def control(
ControlledGate: controlled version of this gate.
"""
if num_ctrl_qubits == 1:
gate = CHGate(label=label, ctrl_state=ctrl_state)
gate.base_gate.label = self.label
gate = CHGate(label=label, ctrl_state=ctrl_state, _base_label=self.label)
return gate
return super().control(num_ctrl_qubits=num_ctrl_qubits, label=label, ctrl_state=ctrl_state)

Expand Down Expand Up @@ -162,10 +165,21 @@ class CHGate(ControlledGate):
\end{pmatrix}
"""

def __init__(self, label: Optional[str] = None, ctrl_state: Optional[Union[int, str]] = None):
def __init__(
self,
label: Optional[str] = None,
ctrl_state: Optional[Union[int, str]] = None,
_base_label=None,
):
"""Create new CH gate."""
super().__init__(
"ch", 2, [], num_ctrl_qubits=1, label=label, ctrl_state=ctrl_state, base_gate=HGate()
"ch",
2,
[],
num_ctrl_qubits=1,
label=label,
ctrl_state=ctrl_state,
base_gate=HGate(label=_base_label),
)

def _define(self):
Expand Down
12 changes: 8 additions & 4 deletions qiskit/circuit/library/standard_gates/i.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
"""Identity gate."""

from typing import Optional
from qiskit.circuit.gate import Gate
from qiskit.circuit.singleton_gate import SingletonGate
from qiskit.circuit._utils import with_gate_array


@with_gate_array([[1, 0], [0, 1]])
class IGate(Gate):
class IGate(SingletonGate):
r"""Identity gate.
Identity gate corresponds to a single-qubit gate wait cycle,
Expand All @@ -45,9 +45,13 @@ class IGate(Gate):
└───┘
"""

def __init__(self, label: Optional[str] = None):
def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
"""Create new Identity gate."""
super().__init__("id", 1, [], label=label)
if unit is None:
unit = "dt"
super().__init__(
"id", 1, [], label=label, _condition=_condition, duration=duration, unit=unit
)

def inverse(self):
"""Invert this gate."""
Expand Down
12 changes: 8 additions & 4 deletions qiskit/circuit/library/standard_gates/iswap.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@

import numpy as np

from qiskit.circuit.gate import Gate
from qiskit.circuit.singleton_gate import SingletonGate
from qiskit.circuit.quantumregister import QuantumRegister
from qiskit.circuit._utils import with_gate_array

from .xx_plus_yy import XXPlusYYGate


@with_gate_array([[1, 0, 0, 0], [0, 0, 1j, 0], [0, 1j, 0, 0], [0, 0, 0, 1]])
class iSwapGate(Gate):
class iSwapGate(SingletonGate):
r"""iSWAP gate.
A 2-qubit XX+YY interaction.
Expand Down Expand Up @@ -85,9 +85,13 @@ class iSwapGate(Gate):
\end{pmatrix}
"""

def __init__(self, label: Optional[str] = None):
def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
"""Create new iSwap gate."""
super().__init__("iswap", 2, [], label=label)
if unit is None:
unit = "dt"
super().__init__(
"iswap", 2, [], label=label, _condition=_condition, duration=duration, unit=unit
)

def _define(self):
"""
Expand Down
22 changes: 15 additions & 7 deletions qiskit/circuit/library/standard_gates/s.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import numpy

from qiskit.circuit.controlledgate import ControlledGate
from qiskit.circuit.gate import Gate
from qiskit.circuit.singleton_gate 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
Expand All @@ -29,7 +29,7 @@


@with_gate_array(_S_ARRAY)
class SGate(Gate):
class SGate(SingletonGate):
r"""Single qubit S gate (Z**0.5).
It induces a :math:`\pi/2` phase, and is sometimes called the P gate (phase).
Expand Down Expand Up @@ -59,9 +59,13 @@ class SGate(Gate):
Equivalent to a :math:`\pi/2` radian rotation about the Z axis.
"""

def __init__(self, label: Optional[str] = None):
def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
"""Create new S gate."""
super().__init__("s", 1, [], label=label)
if unit is None:
unit = "dt"
super().__init__(
"s", 1, [], label=label, _condition=_condition, duration=duration, unit=unit
)

def _define(self):
"""
Expand Down Expand Up @@ -90,7 +94,7 @@ def power(self, exponent: float):


@with_gate_array(_SDG_ARRAY)
class SdgGate(Gate):
class SdgGate(SingletonGate):
r"""Single qubit S-adjoint gate (~Z**0.5).
It induces a :math:`-\pi/2` phase.
Expand Down Expand Up @@ -120,9 +124,13 @@ class SdgGate(Gate):
Equivalent to a :math:`-\pi/2` radian rotation about the Z axis.
"""

def __init__(self, label: Optional[str] = None):
def __init__(self, label: Optional[str] = None, duration=None, unit=None, _condition=None):
"""Create new Sdg gate."""
super().__init__("sdg", 1, [], label=label)
if unit is None:
unit = "dt"
super().__init__(
"sdg", 1, [], label=label, _condition=_condition, duration=duration, unit=unit
)

def _define(self):
"""
Expand Down
Loading

0 comments on commit e62c86b

Please sign in to comment.