-
Notifications
You must be signed in to change notification settings - Fork 569
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add decomposition to Controlled
#2938
Conversation
Hello. You may have forgotten to update the changelog!
|
Codecov Report
@@ Coverage Diff @@
## master #2938 +/- ##
=======================================
Coverage 99.65% 99.65%
=======================================
Files 266 266
Lines 22332 22364 +32
=======================================
+ Hits 22255 22287 +32
Misses 77 77
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
I don't have much time for a full review, but noticed the following: >>> qml.ops.Controlled(qml.PauliRot(3, "XX", [0, 4]), 2).decomposition()
[C(Hadamard)(wires=[2, 0]),
C(Hadamard)(wires=[2, 4]),
C(MultiRZ)(3, wires=[2, 0, 4]),
C(Hadamard)(wires=[2, 0]),
C(Hadamard)(wires=[2, 4])] Shouldn't |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, @albi3ro! 🎉 Just had a few minor questions and a tiny suggestion.
The new _controlled
syntax using a return value looks much better!
@AlbertMitjans I want to add more decomposition algorithms in the future, but that would take a bit more work than just translating over what |
Co-authored-by: David Wierichs <davidwierichs@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @albi3ro 🎉
Just the disordered wires test case question is open from my side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good @albi3ro, I can see now how annoying these in place inversions are haha!
Currently,
ControlledOperation
created byctrl
has some very basic decomposition behaviour. But it at least has more decomposition behaviour than the non-integratedControlled
.This PR makes
Controlled
reach feature-parity withControlledOperation
and will allow us to replace it.