-
Notifications
You must be signed in to change notification settings - Fork 575
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 support for easily creating expansion functions given a device #1760
Conversation
…nnylane into restructure-metric_tensor
…nnylane into restructure-metric_tensor
Co-authored-by: Josh Izaac <josh146@gmail.com>
…nnylane into restructure-metric_tensor
Hello. You may have forgotten to update the changelog!
|
@dwierichs this is an idea I had while reviewing #1743 🙂 |
Codecov Report
@@ Coverage Diff @@
## master #1760 +/- ##
=======================================
Coverage 98.91% 98.91%
=======================================
Files 207 207
Lines 15617 15629 +12
=======================================
+ Hits 15448 15460 +12
Misses 169 169
Continue to review full report at Codecov.
|
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.
Nice addition! This should make using create_expand_fn
much nicer for users.
@@ -36,6 +41,8 @@ def create_expand_fn(depth, stop_at, docstring=None): | |||
``stop_at(obj)``, where ``obj`` is a *queueable* PennyLane object such as | |||
:class:`~.Operation` or :class:`~.MeasurementProcess`. It must return a | |||
boolean, indicating if the expansion should stop at this object. | |||
device (.Device): Ensure that the expanded tape only uses native gates of the | |||
given device. | |||
docstring (str): docstring for the generated expansion function | |||
|
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.
Maybe the docstring should mention in which order the expansions via stop_at
and device
are carried out :)
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.
I actually spent a long time thinking about this! Is the order chosen in this PR the correct order?
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.
I am not sure. On one hand, the device
decomposition can create gates that are not in accordance with the stop_at
criterion (although I am not sure what a realistic scenario for this would be), on the other hand, it would be annoying to provide a device
to create_expand_fn
and receive a tape that does not run on that device (which could happen if we switch the order).
Maybe the latter point is the weaker one and we should actually switch the order, because in PL in general users leave decomposition to device-compatible operations to the backend.
Finally, one could consider forcing both criteria to be satisfied at the same time by transforming the stopping criterion of the device into a BooleanFn
and merging it with stop_at
via &
.
Co-authored-by: David Wierichs <davidwierichs@gmail.com>
pennylane/transforms/tape_expand.py
Outdated
|
||
if device is not None: | ||
tape = device.expand_fn(tape, max_expansion=_depth) | ||
# _update_trainable_params(tape) |
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.
@dwierichs your solution above re: only updating trainable parameters only once, after the with
statement, seems to cause issues; I get > 200 test failures 😆
It seems the only combinations that allow the tests to pass are:
- Not updating
- Updating after every expand. I think this makes sense, since if you are doing multiple tape expansions, and later ones have a stopping criterion that depends on trainable parameters, this will need to be updated inbetween each expansion!
For now, I've added it back after each expansion but commented it out.
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.
Oh, haha, yes that makes sense :) Did not think about the interaction of expands and setting the trainable params!
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.
Can't find anything to criticize, looks good to me :)
The only pending decision would be regarding the order/merging of device
expand and stop_at
expansion, I think.
Co-authored-by: David Wierichs <davidwierichs@gmail.com>
Context: Comments on a recent blog post made me realize that there is no intuitive way in PennyLane to, given a device, construct an expansion function to expand circuits down to the native gate set of that device.
Description of the Change: The recently added
create_expand_fn
is modified, to accept a stopping criterion and/or a device.Benefits: East to create an expansion function using a device as a criterion.
Possible Drawbacks: n/a
Related GitHub Issues: n/a