Update cancel_inverses to have better support for Adjoint
#6752
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6752 +/- ##
==========================================
- Coverage 99.60% 99.60% -0.01%
==========================================
Files 476 476
Lines 45210 45196 -14
==========================================
- Hits 45033 45019 -14
Misses 177 177 ☔ View full report in Codecov by Sentry. |
| """Checks if two operators are equal up to class, data, and hyperparameters""" | ||
| return ( | ||
| op1.__class__ is op2.__class__ | ||
| and (op1.data == op2.data) | ||
| and (op1.hyperparameters == op2.hyperparameters) | ||
| and (op1.wires == op2.wires) | ||
| ) |
There was a problem hiding this comment.
Can we just use qml.equal now?
There was a problem hiding this comment.
qml.equal checks for wire equality as well, we want to compare all attributes except for wires.
There was a problem hiding this comment.
Potentially rename this function to _non_wires_equality?
There was a problem hiding this comment.
Should we instead update our definition of "equal" to take into consideration operations that are symmetric?
There was a problem hiding this comment.
I changed the name of the function locally.
Should we instead update our definition of "equal" to take into consideration operations that are symmetric?
I feel like this is a tangential discussion to object vs numerical equality. It certainly shouldn't be hard to integrate wire symmetry into qml.equal since we already have Attributes containing a set of such ops. Maybe worth including more people in this discussion. Could be a potential "good first issue" if we decide it's worthwhile.
There was a problem hiding this comment.
I can't comment on that part of the file for some reason but the example in the transform docstring could be updated as well.
import pennylane as qml
@qml.transforms.cancel_inverses
@qml.qnode(device=qml.device('default.qubit'))
def circuit(x, y, z):
qml.Hadamard(wires=0)
qml.Hadamard(wires=1)
qml.Hadamard(wires=0)
qml.RX(x, wires=2)
qml.RY(y, wires=1)
qml.X(1)
qml.RZ(z, wires=0)
qml.RX(y, wires=2)
qml.CNOT(wires=[0, 2])
qml.X(1)
return qml.expval(qml.Z(0))
>>> circuit(0.1, 0.2, 0.3)
1.0
# 0.999999999 in the example
Co-authored-by: Andrija Paurevic <[email protected]>
| are_inverses_without_wires = ( | ||
| isinstance(op2, Adjoint) | ||
| and op1.__class__ == op2.base.__class__ | ||
| and op1.data == op2.base.data |
There was a problem hiding this comment.
How is this not raising errors? I'm actually a bit hesitant about directly comparing numeric data like this.
There was a problem hiding this comment.
We don't have a lot of interface testing for cancel_inverses, but using equality for floating point data is fine as long as any arithmetic ops aren't applied to the data. The only issue I see with the use of == is that we will not have any tolerance for comparison, leading to potentially cancellable ops not being cancelled.
There was a problem hiding this comment.
Maybe we can add a tol parameter to cancel_inverses for comparing numerical data. 🤔
|
@mudit2812 I added this PR in the |
|
@PietropaoloFrisoni Since it's feature freeze, I'll move it out of the 0.40 milestone. |
Context:
cancel_inversesdoes not cancelAdjointops if the ops to be cancelled are symmetric on all wires. This PR fixes that, and also cleans up the code incancel_inverses.pya bit.Description of the Change:
_ops_equalfor wires as we check for wire equality/symmetry later._can_cancel_opsfunction, which can be used by bothcancel_inversesandCancelInversesInterpreter.Benefits:
cancel_inversesis better at cancellingAdjointops.Possible Drawbacks:
Related GitHub Issues:
#6729
[sc-80821]