-
Notifications
You must be signed in to change notification settings - Fork 586
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 batch_partial implementation #2585
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2585 +/- ##
=======================================
Coverage 99.58% 99.58%
=======================================
Files 244 245 +1
Lines 19673 19724 +51
=======================================
+ Hits 19591 19642 +51
Misses 82 82
Continue to review full report at Codecov.
|
Thank you @eddddddy! 🎉 Will be leaving a more detailed review during the first half of tomorrow. Hi @dwierichs, just a ping to see if the main UI here is looking good. 👍 |
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.
Looking really nice, thanks @eddddddy! Cool addition to support callables as preprocessing :)
Only have some minor confusions/questions regarding the doc string.
Co-authored-by: David Wierichs <davidwierichs@gmail.com>
Co-authored-by: David Wierichs <davidwierichs@gmail.com>
Co-authored-by: David Wierichs <davidwierichs@gmail.com>
Co-authored-by: David Wierichs <davidwierichs@gmail.com>
Hi @dwierichs, thanks for the helpful comments. There are just a couple outstanding that need another look, otherwise I've mostly addressed them. |
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.
Hi @eddddddy, this is looking great overall 🥳 Have a couple of suggestions for refining things, but the overall implementation seems to check out nicely.
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: antalszava <antalszava@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 good @eddddddy! 🙂 🎉 A couple of final smaller suggestions and a question.
Co-authored-by: antalszava <antalszava@gmail.com>
Context:
There are scenarios where we want to execute a circuit multiple times, with some parameters identical and other parameters differing between the executions. It makes sense to be able to batch these executions together.
Description of the Change:
Add a new transform
qml.batch_partial
that behaves similarly tofunctools.partial
, but supports an additional batch dimension in the unevaluated parameters. Now we would be able to achieve the desired behaviour by doing something like:More complex scenarios involving callable arguments are also possible. For example,
Benefits:
The user can now add a batch dimension to arguments passed to a partially-evaluated QNode.
Possible Drawbacks:
None
Related GitHub Issues:
Closes #1881