From 00cea5886c8d2afa11f3ca0dd8aea773abd2844a Mon Sep 17 00:00:00 2001 From: Christina Lee Date: Thu, 18 Jul 2024 11:59:45 -0400 Subject: [PATCH] CircuitGraph allows the same operation instance to occur multiple times (#5907) **Context:** `CircuitGraph` is from the early queuing era, when we could trust that every operation in a circuit was unique, since queuing had that constraint. This is no longer the case, and can cause errors when the same operation is repeated multiple times. For example, in `TrotterProduct.compute_decomposition`. **Description of the Change:** The nodes of the `CircuitGraph` are now the indices into the queue. Indices will be fully unique, even if the same operation occurs multiple times. This is accomplished in conjunction with two new private attributes: * `_nodes_on_wires`: a dictionary mapping a wire to the nodes that are on that wire * `_inds_for_objs`: a dictionary mapping a `WrappedObj` instance to all the indices where that instance occurs. This is a more general successor to `Operator.queue_idx`. By making this information a private attribute on `CircuitGraph`, we won't have conflicts if the same operation instance occurs in multiple different tapes/ graphs. The private properties `_grid` , `_indices`, and `_operation_graph` are removed, as they are no longer needed. The private method `_in_topological_order` is removed, since it was a private method that nothing used. `graph.operations_in_order` and `graph.observable_in_order` now just return `graph.operations` and `graph.observables` respectively, since those lists are already in order. It was basically "sort this list by the order the objects come in the list". They already come in the order that they come in the list? `_depth` and `max_simultaneous_measurements` are now cached properties. The method for calculating the depth is also simplified. Now we simply provide a weight function on the edges to `rx.dag_longest_path_length`. **Benefits:** We can use the same instance multiple times in a `CircuitGraph` object. **Possible Drawbacks:** `CircuitGraph` no longer sets `Operator.queue_idx`. Maybe someone wanted that. The nodes for `CircuitGraph` are now integer indices, rather than operations. So anyone relying on the individual nodes will see a change in behavior. **Related GitHub Issues:** Fixes #4406 [sc-42719] --------- Co-authored-by: Astral Cai Co-authored-by: Mudit Pandey Co-authored-by: Cristian Emiliano Godinez Ramirez <57567043+EmilianoG-byte@users.noreply.github.com> --- .github/workflows/core_tests_durations.json | 1 - doc/development/deprecations.rst | 4 + doc/releases/changelog-dev.md | 12 + pennylane/circuit_graph.py | 373 +++++++++----------- pennylane/gradients/gradient_transform.py | 8 +- pennylane/gradients/parameter_shift_cv.py | 6 +- pennylane/operation.py | 1 - pennylane/ops/op_math/composite.py | 1 - pennylane/ops/op_math/symbolicop.py | 1 - pennylane/templates/subroutines/trotter.py | 5 +- tests/circuit_graph/test_circuit_graph.py | 167 ++++----- tests/ops/op_math/test_composite.py | 5 - tests/ops/op_math/test_prod.py | 1 - tests/ops/op_math/test_sprod.py | 1 - tests/ops/op_math/test_symbolic_op.py | 1 - tests/pulse/test_parametrized_evolution.py | 1 - 16 files changed, 278 insertions(+), 310 deletions(-) diff --git a/.github/workflows/core_tests_durations.json b/.github/workflows/core_tests_durations.json index bd00edf8f6f..7558d3087e6 100644 --- a/.github/workflows/core_tests_durations.json +++ b/.github/workflows/core_tests_durations.json @@ -13182,7 +13182,6 @@ "ops/op_math/test_composite.py::TestConstruction::test_map_wires[True-expected_overlapping_ops1]": 0.001044666045345366, "ops/op_math/test_composite.py::TestConstruction::test_ndim_params_raises_error": 0.0008577080443501472, "ops/op_math/test_composite.py::TestConstruction::test_parameters": 0.0009155830484814942, - "ops/op_math/test_composite.py::TestConstruction::test_queue_idx": 0.0008379160426557064, "ops/op_math/test_composite.py::TestConstruction::test_raise_error_fewer_than_2_operands": 0.0010355000267736614, "ops/op_math/test_composite.py::TestConstruction::test_tensor_and_hamiltonian_converted": 0.0014357499894686043, "ops/op_math/test_composite.py::TestMscMethods::test_copy[ops_lst0]": 0.0009615410235710442, diff --git a/doc/development/deprecations.rst b/doc/development/deprecations.rst index 768527a7079..43b1bd050f1 100644 --- a/doc/development/deprecations.rst +++ b/doc/development/deprecations.rst @@ -65,6 +65,10 @@ Other deprecations Completed deprecation cycles ---------------------------- +* ``queue_idx`` attribute has been removed from the ``Operator``, ``CompositeOp``, and ``SymboliOp`` classes. Instead, the index is now stored as the label of the ``CircuitGraph.graph`` nodes. + + - Deprecated in v0.38 + - Removed in v0.38 * ``qml.from_qasm`` no longer removes measurements from the QASM code. Use ``measurements=[]`` to remove measurements from the original circuit. diff --git a/doc/releases/changelog-dev.md b/doc/releases/changelog-dev.md index 0b18c0c68e4..96255721987 100644 --- a/doc/releases/changelog-dev.md +++ b/doc/releases/changelog-dev.md @@ -57,6 +57,14 @@

Breaking changes 💔

+* The `CircuitGraph.graph` rustworkx graph now stores indices into the circuit as the node labels, + instead of the operator/ measurement itself. This allows the same operator to occur multiple times in + the circuit. + [(#5907)](https://github.com/PennyLaneAI/pennylane/pull/5907) + +* `queue_idx` attribute has been removed from the `Operator`, `CompositeOp`, and `SymboliOp` classes. + [(#6005)](https://github.com/PennyLaneAI/pennylane/pull/6005) + * ``qml.from_qasm`` no longer removes measurements from the QASM code. Use ``measurements=[]`` to remove measurements from the original circuit. [(#5982)](https://github.com/PennyLaneAI/pennylane/pull/5982) @@ -81,6 +89,9 @@

Bug fixes 🐛

+* `CircuitGraph` can now handle circuits with the same operation instance occuring multiple times. + [(#5907)](https://github.com/PennyLaneAI/pennylane/pull/5907) + * `qml.QSVT` is updated to store wire order correctly. [(#5959)](https://github.com/PennyLaneAI/pennylane/pull/5959) @@ -91,6 +102,7 @@ * `qml.AmplitudeEmbedding` has better support for features using low precision integer data types. [(#5969)](https://github.com/PennyLaneAI/pennylane/pull/5969) +

Contributors ✍️

This release contains contributions from (in alphabetical order): diff --git a/pennylane/circuit_graph.py b/pennylane/circuit_graph.py index 69814d865ed..c93be7f74c4 100644 --- a/pennylane/circuit_graph.py +++ b/pennylane/circuit_graph.py @@ -15,42 +15,23 @@ This module contains the CircuitGraph class which is used to generate a DAG (directed acyclic graph) representation of a quantum circuit from an Operator queue. """ -from collections import namedtuple - -# pylint: disable=too-many-branches,too-many-arguments,too-many-instance-attributes -from numbers import Number +from collections import defaultdict, namedtuple +from functools import cached_property +from typing import List, Optional, Union import numpy as np import rustworkx as rx from pennylane.measurements import MeasurementProcess +from pennylane.operation import Observable, Operator +from pennylane.ops.identity import I +from pennylane.queuing import QueuingManager, WrappedObj from pennylane.resource import ResourcesOperation +from pennylane.wires import Wires -def _by_idx(x): - """Sorting key for Operators: queue index aka temporal order. - - Args: - x (Operator): node in the circuit graph - Returns: - int: sorting key for the node - """ - return x.queue_idx - - -def _is_observable(x): - """Predicate for deciding if an Operator instance is an observable. - - .. note:: - Currently some :class:`Observable` instances are not observables in this sense, - since they can be used as gates as well. - - Args: - x (Operator): node in the circuit graph - Returns: - bool: True iff x is an observable - """ - return isinstance(x, MeasurementProcess) +def _get_wires(obj, all_wires): + return all_wires if len(obj.wires) == 0 else obj.wires Layer = namedtuple("Layer", ["ops", "param_inds"]) @@ -74,6 +55,25 @@ def _is_observable(x): """ +def _construct_graph_from_queue(queue, all_wires): + inds_for_objs = defaultdict(list) # dict from wrappedobjs to all indices for the objs + nodes_on_wires = defaultdict(list) # wire to list of nodes + + graph = rx.PyDiGraph(multigraph=False) + + for i, obj in enumerate(queue): + inds_for_objs[WrappedObj(obj)].append(i) + + obj_node = graph.add_node(i) + for w in _get_wires(obj, all_wires): + if w in nodes_on_wires: + graph.add_edge(nodes_on_wires[w][-1], obj_node, "") + nodes_on_wires[w].append(obj_node) + + return graph, inds_for_objs, nodes_on_wires + + +# pylint: disable=too-many-instance-attributes, too-many-public-methods class CircuitGraph: """Represents a quantum circuit as a directed acyclic graph. @@ -84,85 +84,40 @@ class CircuitGraph: Args: ops (Iterable[.Operator]): quantum operators constituting the circuit, in temporal order - obs (Iterable[.MeasurementProcess]): terminal measurements, in temporal order + obs (List[Union[MeasurementProcess, Observable]]): terminal measurements, in temporal order wires (.Wires): The addressable wire registers of the device that will be executing this graph - par_info (list[dict]): Parameter information. For each index, the entry is a dictionary containing an operation + par_info (Optional[list[dict]]): Parameter information. For each index, the entry is a dictionary containing an operation and an index into that operation's parameters. - trainable_params (set[int]): A set containing the indices of parameters that support + trainable_params (Optional[set[int]]): A set containing the indices of parameters that support differentiability. The indices provided match the order of appearence in the quantum circuit. """ - # pylint: disable=too-many-public-methods - - def __init__(self, ops, obs, wires, par_info=None, trainable_params=None): + # pylint: disable=too-many-arguments + def __init__( + self, + ops: list[Union[Operator, MeasurementProcess]], + obs: List[Union[MeasurementProcess, Observable]], + wires: Wires, + par_info: Optional[list[dict]] = None, + trainable_params: Optional[set[int]] = None, + ): self._operations = ops self._observables = obs self.par_info = par_info self.trainable_params = trainable_params - queue = ops + obs + self._queue = ops + obs - self._depth = None - - self._grid = {} - """dict[int, list[Operator]]: dictionary representing the quantum circuit as a grid. - Here, the key is the wire number, and the value is a list containing the operators on that wire. - """ - - self._indices = {} - # Store indices for the nodes of the DAG here - - self.wires = wires + self.wires = Wires(wires) """Wires: wires that are addressed in the operations. Required to translate between wires and indices of the wires on the device.""" self.num_wires = len(wires) """int: number of wires the circuit contains""" - for k, op in enumerate(queue): - # meas_wires = wires or None # cannot use empty wire list in MeasurementProcess - op.queue_idx = k # store the queue index in the Operator - - for w in wires if len(op.wires) == 0 else op.wires: - # get the index of the wire on the device - wire = wires.index(w) - # add op to the grid, to the end of wire w - self._grid.setdefault(wire, []).append(op) - - # TODO: State preparations demolish the incoming state entirely, and therefore should have no incoming edges. - - self._graph = rx.PyDiGraph( - multigraph=False - ) #: rx.PyDiGraph: DAG representation of the quantum circuit - - # Iterate over each (populated) wire in the grid - for wire in self._grid.values(): - # Add the first operator on the wire to the graph - # This operator does not depend on any others - - # Check if wire[0] in self._grid.values() - # is already added to the graph; this - # condition avoids adding new nodes with - # the same value but different indexes - if all(wire[0] is not op for op in self._graph.nodes()): - _ind = self._graph.add_node(wire[0]) - self._indices.setdefault(id(wire[0]), _ind) - - for i in range(1, len(wire)): - # For subsequent operators on the wire: - if all(wire[i] is not op for op in self._graph.nodes()): - # Add them to the graph if they are not already - # in the graph (multi-qubit operators might already have been placed) - _ind = self._graph.add_node(wire[i]) - self._indices.setdefault(id(wire[i]), _ind) - - # Create an edge between this and the previous operator - # There isn't any default value for the edge-data in - # rx.PyDiGraph.add_edge(); this is set to an empty string - self._graph.add_edge(self._indices[id(wire[i - 1])], self._indices[id(wire[i])], "") - - # For computing depth; want only a graph with the operations, not - # including the observables - self._operation_graph = None + + self._graph, self._inds_for_objs, self._nodes_on_wires = _construct_graph_from_queue( + self._queue, wires + ) # Required to keep track if we need to handle multiple returned # observables per wire @@ -181,7 +136,7 @@ def print_contents(self): for op in self.observables: print(repr(op)) - def serialize(self): + def serialize(self) -> str: """Serialize the quantum circuit graph based on the operations and observables in the circuit graph and the index of the variables used by them. @@ -223,7 +178,7 @@ def serialize(self): return serialization_string @property - def hash(self): + def hash(self) -> int: """Creating a hash for the circuit graph based on the string generated by serialize. Returns: @@ -240,10 +195,9 @@ def observables_in_order(self): Currently the topological order is determined by the queue index. Returns: - list[Observable]: observables + List[Union[MeasurementProcess, Observable]]: observables """ - nodes = [node for node in self._graph.nodes() if _is_observable(node)] - return sorted(nodes, key=_by_idx) + return self._observables @property def observables(self): @@ -262,8 +216,7 @@ def operations_in_order(self): Returns: list[Operation]: operations """ - nodes = [node for node in self._graph.nodes() if not _is_observable(node)] - return sorted(nodes, key=_by_idx) + return self._operations @property def operations(self): @@ -274,7 +227,7 @@ def operations(self): def graph(self): """The graph representation of the quantum circuit. - The graph has nodes representing :class:`.Operator` instances, + The graph has nodes representing indices into the queue, and directed edges pointing from nodes to their immediate dependents/successors. Returns: @@ -291,9 +244,9 @@ def wire_indices(self, wire): Returns: list[int]: indices of operators on the wire, in temporal order """ - return [op.queue_idx for op in self._grid[wire]] + return self._nodes_on_wires[wire] - def ancestors(self, ops): + def ancestors(self, ops, sort=False): """Ancestors of a given set of operators. Args: @@ -302,14 +255,25 @@ def ancestors(self, ops): Returns: list[Operator]: ancestors of the given operators """ - # rx.ancestors() returns node indices instead of node-values - all_indices = set().union(*(rx.ancestors(self._graph, self._indices[id(o)]) for o in ops)) - double_op_indices = set(self._indices[id(o)] for o in ops) - ancestor_indices = all_indices - double_op_indices - - return list(self._graph.get_node_data(n) for n in ancestor_indices) - - def descendants(self, ops): + if isinstance(ops, (Operator, MeasurementProcess)): + raise ValueError( + "CircuitGraph.ancestors accepts an iterable of" + " operators and measurements, not operators and measurements themselves." + ) + if any(len(self._inds_for_objs[WrappedObj(op)]) > 1 for op in ops): + raise ValueError( + "Cannot calculate ancestors for an operator that occurs multiple times." + ) + ancestors = set() + for op in ops: + ind = self._inds_for_objs[WrappedObj(op)][0] + op_ancestors = rx.ancestors(self._graph, ind) + ancestors.update(set(op_ancestors)) + if sort: + ancestors = sorted(ancestors) + return [self._queue[ind] for ind in ancestors] + + def descendants(self, ops, sort=False): """Descendants of a given set of operators. Args: @@ -318,25 +282,23 @@ def descendants(self, ops): Returns: list[Operator]: descendants of the given operators """ - # rx.descendants() returns node indices instead of node-values - all_indices = set().union(*(rx.descendants(self._graph, self._indices[id(o)]) for o in ops)) - double_op_indices = set(self._indices[id(o)] for o in ops) - ancestor_indices = all_indices - double_op_indices - - return list(self._graph.get_node_data(n) for n in ancestor_indices) - - def _in_topological_order(self, ops): - """Sorts a set of operators in the circuit in a topological order. - - Args: - ops (Iterable[Operator]): set of operators in the circuit - - Returns: - Iterable[Operator]: same set of operators, topologically ordered - """ - G = self._graph.subgraph(list(self._indices[id(o)] for o in ops)) - indexes = rx.topological_sort(G) - return list(G[x] for x in indexes) + if isinstance(ops, (Operator, MeasurementProcess)): + raise ValueError( + "CircuitGraph.descendants accepts an iterable of" + " operators and measurements, not operators and measurements themselves." + ) + if any(len(self._inds_for_objs[WrappedObj(op)]) > 1 for op in ops): + raise ValueError( + "cannot calculate decendents for an operator that occurs multiple times." + ) + descendants = set() + for op in ops: + ind = self._inds_for_objs[WrappedObj(op)][0] + op_descendants = rx.descendants(self._graph, ind) + descendants.update(set(op_descendants)) + if sort: + descendants = sorted(descendants) + return [self._queue[ind] for ind in descendants] def ancestors_in_order(self, ops): """Operator ancestors in a topological order. @@ -349,7 +311,7 @@ def ancestors_in_order(self, ops): Returns: list[Operator]: ancestors of the given operators, topologically ordered """ - return sorted(self.ancestors(ops), key=_by_idx) # an abitrary topological order + return self.ancestors(ops, sort=True) def descendants_in_order(self, ops): """Operator descendants in a topological order. @@ -362,7 +324,7 @@ def descendants_in_order(self, ops): Returns: list[Operator]: descendants of the given operators, topologically ordered """ - return sorted(self.descendants(ops), key=_by_idx) + return self.descendants(ops, sort=True) def nodes_between(self, a, b): r"""Nodes on all the directed paths between the two given nodes. @@ -439,84 +401,74 @@ def update_node(self, old, new): Raises: ValueError: if the new :class:`~.Operator` does not act on the same wires as the old one """ - # NOTE Does not alter the graph edges in any way. variable_deps is not changed, _grid is not changed. Dangerous! + # NOTE Does not alter the graph edges in any way. variable_deps is not changed, Dangerous! if new.wires != old.wires: raise ValueError("The new Operator must act on the same wires as the old one.") - new.queue_idx = old.queue_idx - self._graph[self._indices[id(old)]] = new - index = self._indices.pop(id(old)) - self._indices[id(new)] = index + self._inds_for_objs[WrappedObj(new)] = self._inds_for_objs.pop(WrappedObj(old)) - self._operations = self.operations_in_order - self._observables = self.observables_in_order + for i, op in enumerate(self._operations): + if op is old: + self._operations[i] = new + for i, mp in enumerate(self._observables): + if mp is old: + self._observables[i] = new + for i, obj in enumerate(self._queue): + if obj is old: + self._queue[i] = new def get_depth(self): """Depth of the quantum circuit (longest path in the DAG).""" + return self._depth + + @cached_property + def _depth(self): # If there are no operations in the circuit, the depth is 0 if not self.operations: - self._depth = 0 - - # If there are operations but depth is uncomputed, compute the truncated graph - # with only the operations, and return the longest path + 1 (since the path is - # expressed in terms of edges, and we want it in terms of nodes). - if self._depth is None and self.operations: - if self._operation_graph is None: - self._operation_graph = self._graph.subgraph( - list(self._indices[id(node)] for node in self.operations) - ) - self._extend_graph(self._operation_graph) - self._depth = ( - rx.dag_longest_path_length( - self._operation_graph, weight_fn=lambda _, __, w: self._weight_func(w) - ) - + 1 - ) - return self._depth + return 0 + with QueuingManager.stop_recording(): + ops_with_initial_I = [ + I(self.wires) + ] + self.operations # add identity wire to end the graph + operation_graph, _, _ = _construct_graph_from_queue(ops_with_initial_I, self.wires) + + # pylint: disable=unused-argument + def weight_fn(in_idx, out_idx, w): + out_op = ops_with_initial_I[out_idx] + if isinstance(out_op, ResourcesOperation): + return out_op.resources().depth + return 1 + + return rx.dag_longest_path_length(operation_graph, weight_fn=weight_fn) + + def has_path_idx(self, a_idx: int, b_idx: int) -> bool: + """Checks if a path exists between the two given nodes. - @staticmethod - def _weight_func(weight): - """If weight is a number, use it!""" - if isinstance(weight, Number): - return weight - return 1 + Args: + a_idx (int): initial node index + b_idx (int): final node index - def _extend_graph(self, graph: rx.PyDiGraph) -> rx.PyDiGraph: - """Extend graph to account for custom depth operations""" - custom_depth_node_dict = {} - for op in self.operations: - if isinstance(op, ResourcesOperation) and (d := op.resources().depth) > 1: - custom_depth_node_dict[graph.nodes().index(op)] = d - - def _link_graph(target_index, sub_graph, node_index): - """Link incoming and outgoing edges for the initial node to the sub-graph""" - if target_index == node_index: - return sub_graph.nodes().index(f"{node_index}.0") - return sub_graph.nodes().index(f"{node_index}.1") - - for node_index, depth in custom_depth_node_dict.items(): - # Construct sub_graph: - sub_graph = rx.PyDiGraph() - source_node, target_node = (f"{node_index}.0", f"{node_index}.1") - - sub_graph.add_node(source_node) - sub_graph.add_node(target_node) - - sub_graph.add_edge( - sub_graph.nodes().index(source_node), - sub_graph.nodes().index(target_node), - depth - 1, # set edge weight as depth - 1 - ) + Returns: + bool: returns ``True`` if a path exists + """ + if a_idx == b_idx: + return True - graph.substitute_node_with_subgraph( - node_index, - sub_graph, - lambda _, t, __: _link_graph( - t, sub_graph, node_index # pylint: disable=cell-var-from-loop - ), + return ( + len( + rx.digraph_dijkstra_shortest_paths( + self._graph, + a_idx, + b_idx, + weight_fn=None, + default_weight=1.0, + as_undirected=False, + ) ) + != 0 + ) - def has_path(self, a, b): + def has_path(self, a, b) -> bool: """Checks if a path exists between the two given nodes. Args: @@ -526,15 +478,22 @@ def has_path(self, a, b): Returns: bool: returns ``True`` if a path exists """ + if a is b: return True + if any(len(self._inds_for_objs[WrappedObj(o)]) > 1 for o in (a, b)): + raise ValueError( + "CircuitGraph.has_path does not work with operations that have been repeated. " + "Consider using has_path_idx instead." + ) + return ( len( rx.digraph_dijkstra_shortest_paths( self._graph, - self._indices[id(a)], - self._indices[id(b)], + self._inds_for_objs[WrappedObj(a)][0], + self._inds_for_objs[WrappedObj(b)][0], weight_fn=None, default_weight=1.0, as_undirected=False, @@ -543,7 +502,7 @@ def has_path(self, a, b): != 0 ) - @property + @cached_property def max_simultaneous_measurements(self): """Returns the maximum number of measurements on any wire in the circuit graph. @@ -570,15 +529,11 @@ def max_simultaneous_measurements(self): Returns: int: the maximum number of measurements """ - if self._max_simultaneous_measurements is None: - all_wires = [] - - for obs in self.observables: - all_wires.extend(obs.wires.tolist()) - - a = np.array(all_wires) - _, counts = np.unique(a, return_counts=True) - self._max_simultaneous_measurements = ( - counts.max() if counts.size != 0 else 1 - ) # qml.state() will result in an empty array - return self._max_simultaneous_measurements + all_wires = [] + + for obs in self.observables: + all_wires.extend(obs.wires.tolist()) + + a = np.array(all_wires) + _, counts = np.unique(a, return_counts=True) + return counts.max() if counts.size != 0 else 1 # qml.state() will result in an empty array diff --git a/pennylane/gradients/gradient_transform.py b/pennylane/gradients/gradient_transform.py index 223ffe1f43b..be3595a1d85 100644 --- a/pennylane/gradients/gradient_transform.py +++ b/pennylane/gradients/gradient_transform.py @@ -149,8 +149,12 @@ def _try_zero_grad_from_graph_or_get_grad_method(tape, param_index, use_graph=Tr par_info = tape.par_info[param_index] if use_graph: - op_or_mp = tape[par_info["op_idx"]] - if not any(tape.graph.has_path(op_or_mp, mp) for mp in tape.measurements): + op_or_mp_idx = par_info["op_idx"] + n_ops = len(tape.operations) + if not any( + tape.graph.has_path_idx(op_or_mp_idx, n_ops + i) + for i, _ in enumerate(tape.measurements) + ): # there is no influence of this operation on any of the observables return "0" diff --git a/pennylane/gradients/parameter_shift_cv.py b/pennylane/gradients/parameter_shift_cv.py index 2eb69e3f168..aa65e5b51a7 100644 --- a/pennylane/gradients/parameter_shift_cv.py +++ b/pennylane/gradients/parameter_shift_cv.py @@ -376,8 +376,10 @@ def second_order_param_shift(tape, dev_wires, argnum=None, shifts=None, gradient B_inv = B.copy() succ = tape.graph.descendants_in_order((op,)) - operation_descendents = itertools.filterfalse(qml.circuit_graph._is_observable, succ) - observable_descendents = filter(qml.circuit_graph._is_observable, succ) + operation_descendents = itertools.filterfalse( + lambda obj: isinstance(obj, MeasurementProcess), succ + ) + observable_descendents = filter(lambda obj: isinstance(obj, MeasurementProcess), succ) for BB in operation_descendents: if not BB.supports_heisenberg: diff --git a/pennylane/operation.py b/pennylane/operation.py index d31933747cb..5b46976e03e 100644 --- a/pennylane/operation.py +++ b/pennylane/operation.py @@ -1089,7 +1089,6 @@ def __init__(self, *params, wires=None, id=None): self._name = self.__class__.__name__ #: str: name of the operator self._id = id - self.queue_idx = None #: int, None: index of the Operator in the circuit queue, or None if not in a queue self._pauli_rep = None # Union[PauliSentence, None]: Representation of the operator as a pauli sentence, if applicable wires_from_args = False diff --git a/pennylane/ops/op_math/composite.py b/pennylane/ops/op_math/composite.py index 38636be6ea9..15d25e1b398 100644 --- a/pennylane/ops/op_math/composite.py +++ b/pennylane/ops/op_math/composite.py @@ -59,7 +59,6 @@ def __init__( self, *operands: Operator, id=None, _pauli_rep=None ): # pylint: disable=super-init-not-called self._id = id - self.queue_idx = None self._name = self.__class__.__name__ self.operands = operands diff --git a/pennylane/ops/op_math/symbolicop.py b/pennylane/ops/op_math/symbolicop.py index c1afeee1300..3053e58c8c8 100644 --- a/pennylane/ops/op_math/symbolicop.py +++ b/pennylane/ops/op_math/symbolicop.py @@ -72,7 +72,6 @@ def __copy__(self): def __init__(self, base, id=None): self.hyperparameters["base"] = base self._id = id - self.queue_idx = None self._pauli_rep = None self.queue() diff --git a/pennylane/templates/subroutines/trotter.py b/pennylane/templates/subroutines/trotter.py index 64c26dc4768..ae5aa6c5031 100644 --- a/pennylane/templates/subroutines/trotter.py +++ b/pennylane/templates/subroutines/trotter.py @@ -438,10 +438,9 @@ def compute_decomposition(*args, **kwargs): ops = kwargs["base"].operands decomp = _recursive_expression(time / n, order, ops)[::-1] * n - unique_decomp = [copy.copy(op) for op in decomp] if qml.QueuingManager.recording(): - for op in unique_decomp: # apply operators in reverse order of expression + for op in decomp: # apply operators in reverse order of expression qml.apply(op) - return unique_decomp + return decomp diff --git a/tests/circuit_graph/test_circuit_graph.py b/tests/circuit_graph/test_circuit_graph.py index df367b7af63..10886f559f2 100644 --- a/tests/circuit_graph/test_circuit_graph.py +++ b/tests/circuit_graph/test_circuit_graph.py @@ -139,31 +139,19 @@ def test_dependence(self, ops, obs): assert len(graph.node_indexes()) == 9 assert len(graph.edges()) == 9 - queue = ops + obs - - # all ops should be nodes in the graph - for k in queue: - assert k in graph.nodes() - - # all nodes in the graph should be ops - # for k in graph.nodes: - for k in graph.nodes(): - assert k is queue[k.queue_idx] - a = set((graph.get_node_data(e[0]), graph.get_node_data(e[1])) for e in graph.edge_list()) - b = set( - (queue[a], queue[b]) - for a, b in [ - (0, 3), - (1, 3), - (2, 4), - (3, 5), - (3, 6), - (4, 5), - (5, 7), - (5, 8), - (6, 8), - ] - ) + a = {(graph.get_node_data(e[0]), graph.get_node_data(e[1])) for e in graph.edge_list()} + + b = { + (0, 3), + (1, 3), + (2, 4), + (3, 5), + (3, 6), + (4, 5), + (5, 7), + (5, 8), + (6, 8), + } assert a == b def test_ancestors_and_descendants_example(self, ops, obs): @@ -182,25 +170,44 @@ def test_ancestors_and_descendants_example(self, ops, obs): descendants = circuit.descendants([queue[6]]) assert descendants == [queue[8]] - def test_in_topological_order_example(self, ops, obs): - """ - Test ``_in_topological_order`` method returns the expected result. - """ - circuit = CircuitGraph(ops, obs, Wires([0, 1, 2])) - - to = circuit._in_topological_order(ops) - - to_expected = [ - qml.RZ(0.35, wires=[2]), - qml.Hadamard(wires=[2]), - qml.RY(0.35, wires=[1]), - qml.RX(0.43, wires=[0]), - qml.CNOT(wires=[0, 1]), - qml.PauliX(wires=[1]), - qml.CNOT(wires=[2, 0]), - ] - - assert str(to) == str(to_expected) + def test_ancestors_and_descendents_repeated_op(self): + """Test ancestors and descendents raises a ValueError is the requested operation occurs more than once.""" + + op = qml.X(0) + ops = [op, qml.Y(0), op, qml.Z(0), op] + graph = CircuitGraph(ops, [], [0, 1, 2]) + + with pytest.raises(ValueError, match=r"operator that occurs multiple times."): + graph.ancestors([op]) + with pytest.raises(ValueError, match=r"operator that occurs multiple times."): + graph.descendants([op]) + with pytest.raises(ValueError, match=r"operator that occurs multiple times."): + graph.ancestors_in_order([op]) + with pytest.raises(ValueError, match=r"operator that occurs multiple times."): + graph.descendants_in_order([op]) + + def test_ancestors_and_descendents_single_op_error(self): + """Test ancestors and descendents raises a ValueError is the requested operation occurs more than once.""" + + op = qml.Z(0) + graph = CircuitGraph([op], [], [0, 1, 2]) + + with pytest.raises( + ValueError, match=r"CircuitGraph.ancestors accepts an iterable of operators" + ): + graph.ancestors(op) + with pytest.raises( + ValueError, match=r"CircuitGraph.descendants accepts an iterable of operators" + ): + graph.descendants(op) + with pytest.raises( + ValueError, match=r"CircuitGraph.ancestors accepts an iterable of operators" + ): + graph.ancestors_in_order(op) + with pytest.raises( + ValueError, match=r"CircuitGraph.descendants accepts an iterable of operators" + ): + graph.descendants_in_order(op) def test_update_node(self, ops, obs): """Changing nodes in the graph.""" @@ -209,6 +216,9 @@ def test_update_node(self, ops, obs): new = qml.RX(0.1, wires=0) circuit.update_node(ops[0], new) assert circuit.operations[0] is new + new_mp = qml.var(qml.Y(0)) + circuit.update_node(obs[0], new_mp) + assert circuit.observables[0] is new_mp def test_update_node_error(self, ops, obs): """Test that changing nodes in the graph may raise an error.""" @@ -300,43 +310,6 @@ def test_max_simultaneous_measurements(self, circ, expected): circuit = qnode.qtape.graph assert circuit.max_simultaneous_measurements == expected - def test_grid_when_sample_no_wires(self): - """A test to ensure the sample operation applies to all wires on the - `CircuitGraph`s grid when none are explicitly provided.""" - - ops = [qml.Hadamard(wires=0), qml.CNOT(wires=[0, 1])] - obs_no_wires = [qml.sample(op=None, wires=None)] - obs_w_wires = [qml.sample(op=None, wires=[0, 1, 2])] - - circuit_no_wires = CircuitGraph(ops, obs_no_wires, wires=Wires([0, 1, 2])) - circuit_w_wires = CircuitGraph(ops, obs_w_wires, wires=Wires([0, 1, 2])) - - sample_w_wires_op = qml.sample(op=None, wires=[0, 1, 2]) - expected_grid_w_wires = { - 0: [ops[0], ops[1], sample_w_wires_op], - 1: [ops[1], sample_w_wires_op], - 2: [sample_w_wires_op], - } - - sample_no_wires_op = qml.sample(op=None, wires=None) - expected_grid_no_wires = { - 0: [ops[0], ops[1], sample_no_wires_op], - 1: [ops[1], sample_no_wires_op], - 2: [sample_no_wires_op], - } - - for key in range(3): - lst_w_wires = circuit_w_wires._grid[key] - lst_no_wires = circuit_no_wires._grid[key] - lst_expected_w_wires = expected_grid_w_wires[key] - lst_expected_no_wires = expected_grid_no_wires[key] - - for el1, el2 in zip(lst_w_wires, lst_expected_w_wires): - qml.assert_equal(el1, el2) - - for el1, el2 in zip(lst_no_wires, lst_expected_no_wires): - qml.assert_equal(el1, el2) - def test_print_contents(self): """Tests if the circuit prints correct.""" ops = [qml.Hadamard(wires=0), qml.CNOT(wires=[0, 1])] @@ -354,6 +327,7 @@ def test_print_contents(self): tape_depth = ( ([qml.PauliZ(0), qml.CNOT([0, 1]), qml.RX(1.23, 2)], 2), + ([qml.X(0)] * 4, 4), ([qml.Hadamard(0), qml.CNOT([0, 1]), CustomOpDepth3(wires=[1, 0])], 5), ( [ @@ -381,3 +355,34 @@ def test_get_depth(self, ops, true_depth): """Test that depth is computed correctly for operations that define a custom depth > 1""" cg = CircuitGraph(ops, [], wires=[0, 1, 2, 3, 4]) assert cg.get_depth() == true_depth + + +def test_has_path(): + """Test has_path and has_path_idx.""" + + ops = [qml.X(0), qml.X(3), qml.CNOT((0, 1)), qml.X(1), qml.X(3)] + graph = CircuitGraph(ops, [], wires=[0, 1, 2, 3, 4, 5]) + + assert graph.has_path(ops[0], ops[2]) + assert graph.has_path_idx(0, 2) + assert not graph.has_path(ops[0], ops[4]) + assert not graph.has_path_idx(0, 4) + + +def test_has_path_repeated_ops(): + """Test has_path and has_path_idx when an operation is repeated.""" + + op = qml.X(0) + ops = [op, qml.CNOT((0, 1)), op, qml.Y(1)] + + graph = CircuitGraph(ops, [], [0, 1, 2, 3]) + + assert graph.has_path_idx(0, 3) + assert graph.has_path_idx(1, 2) + with pytest.raises(ValueError, match="does not work with operations that have been repeated. "): + graph.has_path(op, ops[3]) + with pytest.raises(ValueError, match="does not work with operations that have been repeated. "): + graph.has_path(ops[1], op) + + # still works if they are the same operation. + assert graph.has_path(op, op) diff --git a/tests/ops/op_math/test_composite.py b/tests/ops/op_math/test_composite.py index 0c21c45ed55..22c33f481c8 100644 --- a/tests/ops/op_math/test_composite.py +++ b/tests/ops/op_math/test_composite.py @@ -87,11 +87,6 @@ def test_initialization(self): assert op._name == "ValidOp" assert op._op_symbol == "#" - def test_queue_idx(self): - """Test that queue_idx is None.""" - op = ValidOp(*self.simple_operands) - assert op.queue_idx is None - def test_parameters(self): """Test that parameters are initialized correctly.""" op = ValidOp(qml.RX(9.87, wires=0), qml.Rot(1.23, 4.0, 5.67, wires=1), qml.PauliX(0)) diff --git a/tests/ops/op_math/test_prod.py b/tests/ops/op_math/test_prod.py index 13302dc3e8d..fabdfe61bdc 100644 --- a/tests/ops/op_math/test_prod.py +++ b/tests/ops/op_math/test_prod.py @@ -165,7 +165,6 @@ def test_init_prod_op(self, id): assert prod_op.num_wires == 2 assert prod_op.name == "Prod" assert prod_op.id == id - assert prod_op.queue_idx is None assert prod_op.data == (0.23,) assert prod_op.parameters == [0.23] diff --git a/tests/ops/op_math/test_sprod.py b/tests/ops/op_math/test_sprod.py index 0bcfca4b7ab..176cb54b7b7 100644 --- a/tests/ops/op_math/test_sprod.py +++ b/tests/ops/op_math/test_sprod.py @@ -109,7 +109,6 @@ def test_init_sprod_op(self, test_id): assert sprod_op.num_wires == 1 assert sprod_op.name == "SProd" assert sprod_op.id == test_id - assert sprod_op.queue_idx is None assert sprod_op.data == (3.14, 0.23) assert sprod_op.parameters == [3.14, 0.23] diff --git a/tests/ops/op_math/test_symbolic_op.py b/tests/ops/op_math/test_symbolic_op.py index 2390c00a829..e50df89fdd8 100644 --- a/tests/ops/op_math/test_symbolic_op.py +++ b/tests/ops/op_math/test_symbolic_op.py @@ -52,7 +52,6 @@ def test_intialization(): assert op.base is base assert op.hyperparameters["base"] is base assert op.id == "something" - assert op.queue_idx is None assert op.name == "Symbolic" diff --git a/tests/pulse/test_parametrized_evolution.py b/tests/pulse/test_parametrized_evolution.py index 7888d71673a..2fced214278 100644 --- a/tests/pulse/test_parametrized_evolution.py +++ b/tests/pulse/test_parametrized_evolution.py @@ -155,7 +155,6 @@ def test_init(self, params, coeffs): assert ev.num_wires == AnyWires assert ev.name == "ParametrizedEvolution" assert ev.id is None - assert ev.queue_idx is None exp_params = [] if params is None else params assert qml.math.allequal(ev.data, exp_params)