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 <[email protected]> * Update pennylane/ops/functions/equal.py Co-authored-by: Christina Lee <[email protected]> --------- Co-authored-by: Matthew Silverman <[email protected]> Co-authored-by: Christina Lee <[email protected]>
PennyLaneAI · Aug 2, 2023 · 970508e · 970508e
1 parent 8265e8b
commit 970508e
Show file tree

Hide file tree

Showing 9 changed files with 91 additions and 66 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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