Closed albi3ro closed 1 month ago
Hi @albi3ro and @trbromley,
I’ve submitted PR #5780. Once the PR is merged, could you please assign this issue to me before closing it?
Thank you!
Thanks a lot @Tarun-Kumar07! Really excited to see some unitaryHACK contributions coming in :technologist:
It looks like @EuGig has also opened a PR for this issue:
We like to encourage working together as much as possible if it works for both of you. Would it make sense for @Tarun-Kumar07 to take on #5648 and @EuGig to take on #5644?
Sounds good to me!!
Awesome, what do you think @Tarun-Kumar07?
Hi @trbromley and @EuGig,
I am open to collaboration. While working on #5644, I had to make some changes due to failing unit tests, which also addressed #5648. I attempted to separate the changes, but it seems they need to be in one PR. I have completed the changes for both issues and only waiting for the feedback.
If you feel there are still some gaps, @EuGig, I would be happy to collaborate further to address them. Thanks !!
@EuGig and @albi3ro, I noticed that the comments @albi3ro made about the tests in PR #5778 are similar to the changes I've implemented in my PR #5780. To avoid duplicating efforts, @EuGig and I could collaborate on PR #5780.
What do you guys think? @albi3ro, @trbromley and @EuGig
@EuGig and @albi3ro, I noticed that the comments @albi3ro made about the tests in PR #5778 are similar to the changes I've implemented in my PR #5780. To avoid duplicating efforts, @EuGig and I could collaborate on PR #5780.
What do you guys think? @albi3ro, @trbromley and @EuGig
Thanks @Tarun-Kumar07, that can work! So the proposal is:
If that's good for both of you, then I think it's a great idea.
@trbromley Sounds good to me!!
Hi @albi3ro,
Could you please review PR #5780 when you have a moment?
Thank you!
Hi @trbromley and @albi3ro,
Could you please have a look at PR #5780? The Unitary Hack ends in two days, and I want to get these bounties done :).
And @EuGig, would you like to collaborate as mentioned in the comment above?
Thanks @Tarun-Kumar07 and sorry for the delay. We'll prioritize reviewing your PR.
Hey @EuGig , there were minor comments in my PR #5780 which I have addressed. I am done with changes on my end. Please let me know if there any improvements which we can collaborate on. Thanks !!
Context
PennyLane currently has a
qml.equal
function which can be used to check the equality of operators, measurements, and tapes. While this has been extremely useful in testing and validation, it can be difficult to figure out why two objects aren't equal.This is why we are currently have added the framework for an
assert_equal
function:https://github.com/PennyLaneAI/pennylane/blob/b6e96588f9e3d4236d1f31f73e22566298a22344/pennylane/ops/functions/equal.py#L179
The framework for this function was added in PR #5634 , but the implementation still needs to be updated.
Implementation Details
Comparison currently relies on a
_equal
single dispatch function, with different "kernels" registered for different types of operators. Currently this function returnsTrue
orFalse
. We are changing it to returnTrue
, or a string that explains why the two operators are not equal. This change has already been made forBasisRotation
:https://github.com/PennyLaneAI/pennylane/blob/b6e96588f9e3d4236d1f31f73e22566298a22344/pennylane/ops/functions/equal.py#L605
This task is to update
_equal
forOperator
classes: see_equal_operators
Controlled
classes: see_equal_controlled
Pow
classes: see_equal_pow
Adjoint
classes: see_equal_adjoint
Exp
classes: see_equal_exp
While more single dispatch kernels exist for
_equal
, they will be updated as part of separate tasks.After this PR, we should be able to do something like:
Use your best judgement as to the contents of the string and we will help refine the message if needed.
Additional Requirements
To fulfill this task, a PR should:
_equal
kernels for the above casestest_equal.py
. Existing tests like,test_adjoint_comparison
can be extended, or new tests can be written.