From 970508e209e0cfd3afd5980cb2f3f6f237f099ef Mon Sep 17 00:00:00 2001 From: Mudit Pandey Date: Fri, 28 Jul 2023 14:29:35 -0400 Subject: [PATCH] Changes to remove use of `Operator.__eq__` (#4398) * Testing changes to operator __eq__ * Updated until `nodes_between` * Updated metric tensor, circuit graph * Updating `qml.equal` * Linters * Updated tape tests * Added coverage * Updates changelog * Removed changes to __eq__ and __hash__ * Update pennylane/operation.py Co-authored-by: Matthew Silverman * Update pennylane/ops/functions/equal.py Co-authored-by: Christina Lee --------- Co-authored-by: Matthew Silverman Co-authored-by: Christina Lee --- doc/releases/changelog-dev.md | 3 ++ pennylane/circuit_graph.py | 45 +++++++++++------------ pennylane/ops/functions/equal.py | 10 ++++- pennylane/transforms/metric_tensor.py | 4 +- tests/circuit_graph/test_circuit_graph.py | 2 +- tests/legacy/test_legacy_qscript_old.py | 21 +++++------ tests/ops/functions/test_equal.py | 30 +++++++++++++-- tests/tape/test_qscript.py | 21 +++++------ tests/tape/test_tape.py | 21 +++++------ 9 files changed, 91 insertions(+), 66 deletions(-) diff --git a/doc/releases/changelog-dev.md b/doc/releases/changelog-dev.md index d2f91dff007..93358b94b61 100644 --- a/doc/releases/changelog-dev.md +++ b/doc/releases/changelog-dev.md @@ -88,6 +88,9 @@ * When given a callable, `qml.ctrl` now does its custom pre-processing on all queued operators from the callable. [(#4370)](https://github.com/PennyLaneAI/pennylane/pull/4370) +* PennyLane no longer directly relies on `Operator.__eq__`. + [(#4398)](https://github.com/PennyLaneAI/pennylane/pull/4398) + * If no seed is specified on initialization with `DefaultQubit2`, the local random number generator will be seeded from on the NumPy's global random number generator. [(#4394)](https://github.com/PennyLaneAI/pennylane/pull/4394) diff --git a/pennylane/circuit_graph.py b/pennylane/circuit_graph.py index 86d6d7d6ce2..8f31e0ae278 100644 --- a/pennylane/circuit_graph.py +++ b/pennylane/circuit_graph.py @@ -296,16 +296,14 @@ def ancestors(self, ops): ops (Iterable[Operator]): set of operators in the circuit Returns: - set[Operator]: ancestors of the given operators + list[Operator]: ancestors of the given operators """ - anc = set( - self._graph.get_node_data(n) - for n in set().union( - # rx.ancestors() returns node indexes instead of node-values - *(rx.ancestors(self._graph, self._indices[id(o)]) for o in ops) - ) - ) - return anc - set(ops) + # 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): """Descendants of a given set of operators. @@ -314,16 +312,14 @@ def descendants(self, ops): ops (Iterable[Operator]): set of operators in the circuit Returns: - set[Operator]: descendants of the given operators + list[Operator]: descendants of the given operators """ - des = set( - self._graph.get_node_data(n) - for n in set().union( - # rx.descendants() returns node indexes instead of node-values - *(rx.descendants(self._graph, self._indices[id(o)]) for o in ops) - ) - ) - return des - set(ops) + # 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. @@ -376,13 +372,14 @@ def nodes_between(self, a, b): b (Operator): final node Returns: - set[Operator]: nodes on all the directed paths between a and b + list[Operator]: nodes on all the directed paths between a and b """ A = self.descendants([a]) - A.add(a) + A.append(a) B = self.ancestors([b]) - B.add(b) - return A & B + B.append(b) + + return [B.pop(i) for op1 in A for i, op2 in enumerate(B) if op1 is op2] @property def parametrized_layers(self): @@ -405,7 +402,7 @@ def parametrized_layers(self): # check if any of the dependents are in the # currently assembled layer - if set(current.ops) & sub: + if any(o1 is o2 for o1 in current.ops for o2 in sub): # operator depends on current layer, start a new layer current = Layer([], []) layers.append(current) @@ -525,7 +522,7 @@ def has_path(self, a, b): Returns: bool: returns ``True`` if a path exists """ - if a == b: + if a is b: return True return ( diff --git a/pennylane/ops/functions/equal.py b/pennylane/ops/functions/equal.py index 5c74bd521e8..ab30b71fcba 100644 --- a/pennylane/ops/functions/equal.py +++ b/pennylane/ops/functions/equal.py @@ -15,6 +15,7 @@ This module contains the qml.equal function. """ # pylint: disable=too-many-arguments,too-many-return-statements +from collections.abc import Iterable from functools import singledispatch from typing import Union import pennylane as qml @@ -378,7 +379,14 @@ def _equal_shadow_measurements(op1: ShadowExpvalMP, op2: ShadowExpvalMP, **kwarg """Determine whether two ShadowExpvalMP objects are equal""" wires_match = op1.wires == op2.wires - H_match = op1.H == op2.H + + if isinstance(op1.H, Operator) and isinstance(op2.H, Operator): + H_match = equal(op1.H, op2.H) + elif isinstance(op1.H, Iterable) and isinstance(op2.H, Iterable): + H_match = all(equal(o1, o2) for o1, o2 in zip(op1.H, op2.H)) + else: + return False + k_match = op1.k == op2.k return wires_match and H_match and k_match diff --git a/pennylane/transforms/metric_tensor.py b/pennylane/transforms/metric_tensor.py index fb33f00eafb..cef48b5c3b0 100644 --- a/pennylane/transforms/metric_tensor.py +++ b/pennylane/transforms/metric_tensor.py @@ -671,7 +671,9 @@ def _get_first_term_tapes(layer_i, layer_j, allow_nonunitary, aux_wire): tapes = [] ids = [] # Exclude the backwards cone of layer_i from the backwards cone of layer_j - ops_between_cgens = [op for op in layer_j.pre_ops if op not in layer_i.pre_ops] + ops_between_cgens = [ + op1 for op1 in layer_j.pre_ops if not any(op1 is op2 for op2 in layer_i.pre_ops) + ] # Iterate over differentiated operation in first layer for diffed_op_i, par_idx_i in zip(layer_i.ops, layer_i.param_inds): diff --git a/tests/circuit_graph/test_circuit_graph.py b/tests/circuit_graph/test_circuit_graph.py index cb37c719bc8..d93c1b49b52 100644 --- a/tests/circuit_graph/test_circuit_graph.py +++ b/tests/circuit_graph/test_circuit_graph.py @@ -179,7 +179,7 @@ def test_ancestors_and_descendants_example(self, ops, obs): assert queue[o_idx] in ancestors descendants = circuit.descendants([queue[6]]) - assert descendants == set([queue[8]]) + assert descendants == [queue[8]] def test_in_topological_order_example(self, ops, obs): """ diff --git a/tests/legacy/test_legacy_qscript_old.py b/tests/legacy/test_legacy_qscript_old.py index 096be217fe8..be94dc3cbd0 100644 --- a/tests/legacy/test_legacy_qscript_old.py +++ b/tests/legacy/test_legacy_qscript_old.py @@ -472,10 +472,9 @@ def test_shallow_copy(self): assert copied_qs is not qs # the operations are simply references - assert copied_qs.operations == qs.operations - assert copied_qs.observables == qs.observables - assert copied_qs.measurements == qs.measurements - assert copied_qs.operations[0] is qs.operations[0] + assert all(o1 is o2 for o1, o2 in zip(copied_qs.operations, qs.operations)) + assert all(o1 is o2 for o1, o2 in zip(copied_qs.observables, qs.observables)) + assert all(m1 is m2 for m1, m2 in zip(copied_qs.measurements, qs.measurements)) # operation data is also a reference assert copied_qs.operations[0].wires is qs.operations[0].wires @@ -518,10 +517,9 @@ def test_shallow_copy_with_operations(self, copy_fn): assert copied_qs is not qs # the operations are not references; they are unique objects - assert copied_qs.operations != qs.operations - assert copied_qs.observables != qs.observables - assert copied_qs.measurements != qs.measurements - assert copied_qs.operations[0] is not qs.operations[0] + assert all(o1 is not o2 for o1, o2 in zip(copied_qs.operations, qs.operations)) + assert all(o1 is not o2 for o1, o2 in zip(copied_qs.observables, qs.observables)) + assert all(m1 is not m2 for m1, m2 in zip(copied_qs.measurements, qs.measurements)) # however, the underlying operation data *is still shared* assert copied_qs.operations[0].wires is qs.operations[0].wires @@ -559,10 +557,9 @@ def test_deep_copy(self): assert copied_qs is not qs # the operations are not references - assert copied_qs.operations != qs.operations - assert copied_qs.observables != qs.observables - assert copied_qs.measurements != qs.measurements - assert copied_qs.operations[0] is not qs.operations[0] + assert all(o1 is not o2 for o1, o2 in zip(copied_qs.operations, qs.operations)) + assert all(o1 is not o2 for o1, o2 in zip(copied_qs.observables, qs.observables)) + assert all(m1 is not m2 for m1, m2 in zip(copied_qs.measurements, qs.measurements)) # check that the output dim is identical assert qs.output_dim == copied_qs.output_dim diff --git a/tests/ops/functions/test_equal.py b/tests/ops/functions/test_equal.py index bda02c3272d..f37ad1e8a5c 100644 --- a/tests/ops/functions/test_equal.py +++ b/tests/ops/functions/test_equal.py @@ -127,8 +127,8 @@ qml.expval(qml.PauliX(1)), qml.probs(wires=1), qml.probs(wires=0), - qml.probs(qml.PauliZ(0)), - qml.probs(qml.PauliZ(1)), + qml.probs(op=qml.PauliZ(0)), + qml.probs(op=qml.PauliZ(1)), qml.state(), qml.vn_entropy(wires=0), qml.vn_entropy(wires=0, log_base=np.e), @@ -151,7 +151,31 @@ qml.shadow_expval( H=qml.Hamiltonian( [1.0, 1.0], [qml.PauliX(0) @ qml.PauliX(1), qml.PauliZ(0) @ qml.PauliZ(1)] - ) + ), + k=3, + ), + qml.shadow_expval( + H=[ + qml.Hamiltonian( + [1.0, 1.0], [qml.PauliZ(0) @ qml.PauliZ(1), qml.PauliX(0) @ qml.PauliX(1)] + ) + ], + k=2, + ), + qml.shadow_expval( + H=[ + qml.Hamiltonian( + [1.0, 1.0], [qml.PauliZ(0) @ qml.PauliZ(1), qml.PauliX(0) @ qml.PauliX(1)] + ) + ] + ), + qml.shadow_expval( + H=[ + qml.Hamiltonian( + [1.0, 1.0], [qml.PauliX(0) @ qml.PauliX(1), qml.PauliZ(0) @ qml.PauliZ(1)] + ) + ], + k=3, ), ExpectationMP(eigvals=[1, -1]), ExpectationMP(eigvals=[1, 2]), diff --git a/tests/tape/test_qscript.py b/tests/tape/test_qscript.py index c649b905d2f..dc1e743ccff 100644 --- a/tests/tape/test_qscript.py +++ b/tests/tape/test_qscript.py @@ -482,10 +482,9 @@ def test_shallow_copy(self): assert copied_qs is not qs # the operations are simply references - assert copied_qs.operations == qs.operations - assert copied_qs.observables == qs.observables - assert copied_qs.measurements == qs.measurements - assert copied_qs.operations[0] is qs.operations[0] + assert all(o1 is o2 for o1, o2 in zip(copied_qs.operations, qs.operations)) + assert all(o1 is o2 for o1, o2 in zip(copied_qs.observables, qs.observables)) + assert all(m1 is m2 for m1, m2 in zip(copied_qs.measurements, qs.measurements)) # operation data is also a reference assert copied_qs.operations[0].wires is qs.operations[0].wires @@ -520,10 +519,9 @@ def test_shallow_copy_with_operations(self, copy_fn): assert copied_qs is not qs # the operations are not references; they are unique objects - assert copied_qs.operations != qs.operations - assert copied_qs.observables != qs.observables - assert copied_qs.measurements != qs.measurements - assert copied_qs.operations[0] is not qs.operations[0] + assert all(o1 is not o2 for o1, o2 in zip(copied_qs.operations, qs.operations)) + assert all(o1 is not o2 for o1, o2 in zip(copied_qs.observables, qs.observables)) + assert all(m1 is not m2 for m1, m2 in zip(copied_qs.measurements, qs.measurements)) # however, the underlying operation data *is still shared* assert copied_qs.operations[0].wires is qs.operations[0].wires @@ -550,10 +548,9 @@ def test_deep_copy(self): assert copied_qs is not qs # the operations are not references - assert copied_qs.operations != qs.operations - assert copied_qs.observables != qs.observables - assert copied_qs.measurements != qs.measurements - assert copied_qs.operations[0] is not qs.operations[0] + assert all(o1 is not o2 for o1, o2 in zip(copied_qs.operations, qs.operations)) + assert all(o1 is not o2 for o1, o2 in zip(copied_qs.observables, qs.observables)) + assert all(m1 is not m2 for m1, m2 in zip(copied_qs.measurements, qs.measurements)) assert copied_qs.shots is qs.shots # check that the output dim is identical diff --git a/tests/tape/test_tape.py b/tests/tape/test_tape.py index ce7ca541c7e..6b167173423 100644 --- a/tests/tape/test_tape.py +++ b/tests/tape/test_tape.py @@ -1696,10 +1696,9 @@ def test_shallow_copy(self): assert copied_tape is not tape # the operations are simply references - assert copied_tape.operations == tape.operations - assert copied_tape.observables == tape.observables - assert copied_tape.measurements == tape.measurements - assert copied_tape.operations[0] is tape.operations[0] + assert all(o1 is o2 for o1, o2 in zip(copied_tape.operations, tape.operations)) + assert all(o1 is o2 for o1, o2 in zip(copied_tape.observables, tape.observables)) + assert all(m1 is m2 for m1, m2 in zip(copied_tape.measurements, tape.measurements)) # operation data is also a reference assert copied_tape.operations[0].wires is tape.operations[0].wires @@ -1729,10 +1728,9 @@ def test_shallow_copy_with_operations(self, copy_fn): assert copied_tape is not tape # the operations are not references; they are unique objects - assert copied_tape.operations != tape.operations - assert copied_tape.observables != tape.observables - assert copied_tape.measurements != tape.measurements - assert copied_tape.operations[0] is not tape.operations[0] + assert all(o1 is not o2 for o1, o2 in zip(copied_tape.operations, tape.operations)) + assert all(o1 is not o2 for o1, o2 in zip(copied_tape.observables, tape.observables)) + assert all(m1 is not m2 for m1, m2 in zip(copied_tape.measurements, tape.measurements)) # however, the underlying operation data *is still shared* assert copied_tape.operations[0].wires is tape.operations[0].wires @@ -1758,10 +1756,9 @@ def test_deep_copy(self): assert copied_tape is not tape # the operations are not references - assert copied_tape.operations != tape.operations - assert copied_tape.observables != tape.observables - assert copied_tape.measurements != tape.measurements - assert copied_tape.operations[0] is not tape.operations[0] + assert all(o1 is not o2 for o1, o2 in zip(copied_tape.operations, tape.operations)) + assert all(o1 is not o2 for o1, o2 in zip(copied_tape.observables, tape.observables)) + assert all(m1 is not m2 for m1, m2 in zip(copied_tape.measurements, tape.measurements)) # check that the output dim is identical assert tape.output_dim == copied_tape.output_dim