-
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
[QIM-8] quantum fisher information (new) #2684
Conversation
Hello. You may have forgotten to update the changelog!
|
Codecov Report
@@ Coverage Diff @@
## master #2684 +/- ##
=======================================
Coverage 99.61% 99.61%
=======================================
Files 251 251
Lines 20675 20682 +7
=======================================
+ Hits 20595 20602 +7
Misses 80 80
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.
It looks good 💯 just one minor comment
# TODO: ``hardware`` argument will be obsolete in future releases when ``shots`` can be inferred. | ||
if hardware: |
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.
@eddddddy / @Qottmann: I think we should be able to infer this :) Via something like:
if qnode.device.shots is not None:
I think it might be best to make this change in the feature freeze week, for two reasons:
- Consistency with the rest of PennyLane (where there is no
hardware
option) - Because, more accurately, the logic here is more about 'are shots finite', rather than 'is the device hardware'. There will be cases where you are using a simulator, but will be using finite shots, so this may confuse the reader!
if hardware: | ||
|
||
def wrapper(*args0, **kwargs0): | ||
return 4 * metric_tensor(qnode, *args, **kwargs)(*args0, **kwargs0) |
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.
It is assumed here that qnode
is a QNode, so you'll need to update the docstring to remove mention of the tape!
Context:
Setting up a new PR of the previous (2676) due to many changes that underwent in the previous branches.