Skip to content

Commit

Permalink
Revert memory-owning expr.Var nodes (#11667)
Browse files Browse the repository at this point in the history
This commit is a roll-up reversion of the following commits (PR):

* commit a79e879 (#10977)
* commit ba161e9 (#10974)
* commit 50e8137 (#10944)

This is being done to clear the 1.0 branch of memory-owning `expr.Var`
variables and `Store` instructions.  It should be re-applied once 1.0 is
released.

This wasn't done by individual `revert` operations, because there were
also significant structural changes introduced in those PRs that were
very valid and should be maintained.  Cross-references to `Var` nodes
from other functions have been removed for now.

Making `Var` and `types.Type` hashable is maintained, as is the
`Var.standalone` function, in order to prepare the ground for the
inclusion of proper `Var` nodes in a minor release.
  • Loading branch information
jakelishman authored Jan 29, 2024
1 parent 11a826b commit 052d889
Show file tree
Hide file tree
Showing 17 changed files with 20 additions and 2,166 deletions.
2 changes: 0 additions & 2 deletions qiskit/circuit/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@
InstructionSet
Operation
EquivalenceLibrary
Store
Control Flow Operations
-----------------------
Expand Down Expand Up @@ -377,7 +376,6 @@
from .delay import Delay
from .measure import Measure
from .reset import Reset
from .store import Store
from .parameter import Parameter
from .parametervector import ParameterVector
from .parameterexpression import ParameterExpression
Expand Down
20 changes: 3 additions & 17 deletions qiskit/circuit/classical/expr/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,10 @@
These objects are mutable and should not be reused in a different location without a copy.
The base for dynamic variables is the :class:`Var`, which can be either an arbitrarily typed runtime
variable, or a wrapper around a :class:`.Clbit` or :class:`.ClassicalRegister`.
The entry point from general circuit objects to the expression system is by wrapping the object
in a :class:`Var` node and associating a :class:`~.types.Type` with it.
.. autoclass:: Var
:members: var, name
Similarly, literals used in comparison (such as integers) should be lifted to :class:`Value` nodes
with associated types.
Expand Down Expand Up @@ -92,13 +91,6 @@
.. autofunction:: lift
Typically you should create memory-owning :class:`Var` instances by using the
:meth:`.QuantumCircuit.add_var` method to declare them in some circuit context, since a
:class:`.QuantumCircuit` will not accept an :class:`Expr` that contains variables that are not
already declared in it, since it needs to know how to allocate the storage and how the variable will
be initialized. However, should you want to do this manually, you should use the low-level
:meth:`Var.new` call to safely generate a named variable for usage.
You can manually specify casts in cases where the cast is allowed in explicit form, but may be
lossy (such as the cast of a higher precision :class:`~.types.Uint` to a lower precision one).
Expand Down Expand Up @@ -160,11 +152,6 @@
suitable "key" functions to do the comparison.
.. autofunction:: structurally_equivalent
Some expressions have associated memory locations, and others may be purely temporary.
You can use :func:`is_lvalue` to determine whether an expression has an associated memory location.
.. autofunction:: is_lvalue
"""

__all__ = [
Expand All @@ -177,7 +164,6 @@
"ExprVisitor",
"iter_vars",
"structurally_equivalent",
"is_lvalue",
"lift",
"cast",
"bit_not",
Expand All @@ -197,7 +183,7 @@
]

from .expr import Expr, Var, Value, Cast, Unary, Binary
from .visitors import ExprVisitor, iter_vars, structurally_equivalent, is_lvalue
from .visitors import ExprVisitor, iter_vars, structurally_equivalent
from .constructors import (
lift,
cast,
Expand Down
55 changes: 13 additions & 42 deletions qiskit/circuit/classical/expr/expr.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import abc
import enum
import typing
import uuid

from .. import types

Expand Down Expand Up @@ -111,49 +110,29 @@ def __repr__(self):
class Var(Expr):
"""A classical variable.
These variables take two forms: a new-style variable that owns its storage location and has an
associated name; and an old-style variable that wraps a :class:`.Clbit` or
:class:`.ClassicalRegister` instance that is owned by some containing circuit. In general,
construction of variables for use in programs should use :meth:`Var.new` or
:meth:`.QuantumCircuit.add_var`.
Variables are immutable after construction, so they can be used as dictionary keys."""

__slots__ = ("var", "name")
__slots__ = ("var",)

var: qiskit.circuit.Clbit | qiskit.circuit.ClassicalRegister | uuid.UUID
var: qiskit.circuit.Clbit | qiskit.circuit.ClassicalRegister
"""A reference to the backing data storage of the :class:`Var` instance. When lifting
old-style :class:`.Clbit` or :class:`.ClassicalRegister` instances into a :class:`Var`,
this is exactly the :class:`.Clbit` or :class:`.ClassicalRegister`. If the variable is a
new-style classical variable (one that owns its own storage separate to the old
:class:`.Clbit`/:class:`.ClassicalRegister` model), this field will be a :class:`~uuid.UUID`
to uniquely identify it."""
name: str | None
"""The name of the variable. This is required to exist if the backing :attr:`var` attribute
is a :class:`~uuid.UUID`, i.e. if it is a new-style variable, and must be ``None`` if it is
an old-style variable."""
this is exactly the :class:`.Clbit` or :class:`.ClassicalRegister`."""

def __init__(
self,
var: qiskit.circuit.Clbit | qiskit.circuit.ClassicalRegister | uuid.UUID,
var: qiskit.circuit.Clbit | qiskit.circuit.ClassicalRegister,
type: types.Type,
*,
name: str | None = None,
):
super().__setattr__("type", type)
super().__setattr__("var", var)
super().__setattr__("name", name)

@classmethod
def new(cls, name: str, type: types.Type) -> typing.Self:
"""Generate a new named variable that owns its own backing storage."""
return cls(uuid.uuid4(), type, name=name)

@property
def standalone(self) -> bool:
"""Whether this :class:`Var` is a standalone variable that owns its storage location. If
false, this is a wrapper :class:`Var` around a pre-existing circuit object."""
return isinstance(self.var, uuid.UUID)
"""Whether this :class:`Var` is a standalone variable that owns its storage location.
This is currently always ``False``, but will expand in the future to support memory-owning
storage locations."""
return False

def accept(self, visitor, /):
return visitor.visit_var(self)
Expand All @@ -164,29 +143,21 @@ def __setattr__(self, key, value):
raise AttributeError(f"'Var' object has no attribute '{key}'")

def __hash__(self):
return hash((self.type, self.var, self.name))
return hash((self.type, self.var))

def __eq__(self, other):
return (
isinstance(other, Var)
and self.type == other.type
and self.var == other.var
and self.name == other.name
)
return isinstance(other, Var) and self.type == other.type and self.var == other.var

def __repr__(self):
if self.name is None:
return f"Var({self.var}, {self.type})"
return f"Var({self.var}, {self.type}, name='{self.name}')"
return f"Var({self.var}, {self.type})"

def __getstate__(self):
return (self.var, self.type, self.name)
return (self.var, self.type)

def __setstate__(self, state):
var, type, name = state
var, type = state
super().__setattr__("type", type)
super().__setattr__("var", var)
super().__setattr__("name", name)

def __copy__(self):
# I am immutable...
Expand Down
63 changes: 0 additions & 63 deletions qiskit/circuit/classical/expr/visitors.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,66 +215,3 @@ def structurally_equivalent(
True
"""
return left.accept(_StructuralEquivalenceImpl(right, left_var_key, right_var_key))


class _IsLValueImpl(ExprVisitor[bool]):
__slots__ = ()

def visit_var(self, node, /):
return True

def visit_value(self, node, /):
return False

def visit_unary(self, node, /):
return False

def visit_binary(self, node, /):
return False

def visit_cast(self, node, /):
return False


_IS_LVALUE = _IsLValueImpl()


def is_lvalue(node: expr.Expr, /) -> bool:
"""Return whether this expression can be used in l-value positions, that is, whether it has a
well-defined location in memory, such as one that might be writeable.
Being an l-value is a necessary but not sufficient for this location to be writeable; it is
permissible that a larger object containing this memory location may not allow writing from
the scope that attempts to write to it. This would be an access property of the containing
program, however, and not an inherent property of the expression system.
Examples:
Literal values are never l-values; there's no memory location associated with (for example)
the constant ``1``::
>>> from qiskit.circuit.classical import expr
>>> expr.is_lvalue(expr.lift(2))
False
:class:`~.expr.Var` nodes are always l-values, because they always have some associated
memory location::
>>> from qiskit.circuit.classical import types
>>> from qiskit.circuit import Clbit
>>> expr.is_lvalue(expr.Var.new("a", types.Bool()))
True
>>> expr.is_lvalue(expr.lift(Clbit()))
True
Currently there are no unary or binary operations on variables that can produce an l-value
expression, but it is likely in the future that some sort of "indexing" operation will be
added, which could produce l-values::
>>> a = expr.Var.new("a", types.Uint(8))
>>> b = expr.Var.new("b", types.Uint(8))
>>> expr.is_lvalue(a) and expr.is_lvalue(b)
True
>>> expr.is_lvalue(expr.bit_and(a, b))
False
"""
return node.accept(_IS_LVALUE)
Loading

0 comments on commit 052d889

Please sign in to comment.