-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Tutorial on the gradients framework (fixes #1390) #1463
Conversation
**Summary** Fixes Qiskit#1390 **Details** Updates the gradients tutorial from `opflow.gradients` to `algorithms.gradients` - Use of BaseEstimator and BaseSampler classes from Primitives for gradient evaluation - Demonstration of different methods for gradient evaluation - Application example: VQE - Solved using Estimator, Sampler, and classical optimizer (scipy.minimize) *Comments* - More details on SPSA gradient and Qiskit Primitives can be added
…thms Tutorial on the gradients framework
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
View / edit / reply to this conversation on ReviewNB ElePT commented on 2023-06-01T15:05:57Z I think that the title "Qiskit Gradient Framework using Primitives" is a bit more fitting, "The Gradient Framework" sounds too generic for me.
Also, I like how concise the tutorial goal is, but I think it could even be made into a single sentence, and include a link to the API ref. for the primitives: This tutorial demonstrates the use of the
Finally, if it is not longer useful, could you please remove the comment? It is fine to have some comments while drafting the tutorial, but we should avoid them in the final version :) maxwell04-wq commented on 2023-06-01T19:32:19Z Incorporated. |
View / edit / reply to this conversation on ReviewNB ElePT commented on 2023-06-01T15:05:58Z I would call this section something like "A quick refresher on Qiskit Primitives".
Qiskit Primitives is a new framework for executing quantum jobs. Instead of binding parameters to quantum circuits, the
And I would suggest a few changes to the sentence above, I know that explaining the primitives is still a bit tricky, but to make it better it is important to be precise describing what the primitives do:
The Qiskit Primitives work as an abstraction level between algorithms and (real/simulated) quantum devices. Instead of having to manually deal with tasks such as parameter binding or circuit transpilation, the
Now, for the following section: Qiskit Primitives provides two classess for evaluating the circuit: The The
I would say:
maxwell04-wq commented on 2023-06-01T19:37:37Z Incorporated. |
View / edit / reply to this conversation on ReviewNB ElePT commented on 2023-06-01T15:05:59Z Overall, I really like this section, but to push it to the next level I would restructure it a little bit, and explain the base classes first, include classical gradients and talk about the QGT. This would look like this:
The
Additionally, the module offers reverse gradients for efficient classical computations.
The metrics available are based on the notion of the Quantum Geometric Tensor (QGT). There is a
As well as a Quantum Fisher Information class (QFI) that is initialized with a reference QGT implementation from the above list. maxwell04-wq commented on 2023-06-01T19:40:37Z Incorporated. |
View / edit / reply to this conversation on ReviewNB ElePT commented on 2023-06-01T15:06:00Z I love that you have made a diagram, this shows a lot of effort :) I think we can replace Qiskit Algorithms with Qiskit Primitives at the bottom (as the gradients are built on top of that), and add the Reverse Gradient to the list (I know that I mentioned that a reverse gradient tutorial was optional, we don't have to go into detail, but I think that we should at least name them and show where they fit in the overall structure).
I am not so sure about the "Circuit" in "Circuit Gradients" and "Circuit Metrics", we can maybe drop it. maxwell04-wq commented on 2023-06-01T20:12:42Z Incorporated. |
View / edit / reply to this conversation on ReviewNB ElePT commented on 2023-06-01T15:06:01Z We are trying to move away from having import blocks at the beginning of the tutorial, instead, we encourage to add the relevant imports at the top of the cell where they are used. You can see this, for example, in this tutorial https://qiskit.org/ecosystem/ibm-runtime/tutorials/grover_with_sampler.html maxwell04-wq commented on 2023-06-01T20:28:57Z Incorporated. |
View / edit / reply to this conversation on ReviewNB ElePT commented on 2023-06-01T15:06:02Z
Given that we no longer talk about the difference between First and Second Order gradients, maybe just "Gradients" fits this section better.
Here, and in the following subsections, I think it's important to always be very clear about when we are showing estimator gradients and when we are using sampler gradients. You already do this in the Parameter Shift Gradients section, but in the other sections you don't mention that you are talking about estimator gradients. maxwell04-wq commented on 2023-06-01T20:30:46Z I'm adding a note in the Sampler example that all the following examples will be with estimators. |
View / edit / reply to this conversation on ReviewNB ElePT commented on 2023-06-01T15:06:03Z Citing the spsa paperwould be nice.
maxwell04-wq commented on 2023-06-01T20:35:20Z Done. |
View / edit / reply to this conversation on ReviewNB ElePT commented on 2023-06-01T15:06:04Z similar comment to the one I left above, we should spread out the imports. maxwell04-wq commented on 2023-06-01T20:37:15Z Done. |
View / edit / reply to this conversation on ReviewNB ElePT commented on 2023-06-01T15:06:06Z Line #1. #Make circuit copies for different VQEs
|
View / edit / reply to this conversation on ReviewNB ElePT commented on 2023-06-01T15:06:07Z
This problem can also be tackled with the |
View / edit / reply to this conversation on ReviewNB ElePT commented on 2023-06-01T15:06:08Z Line #3. vqe = SamplingVQE(sampler=sampler, ansatz=wavefunction_1, optimizer=optimizer) I think that the point of this example was to show the use of a sampler gradient, right? but no custom gradient is set here maxwell04-wq commented on 2023-06-01T20:41:21Z
|
Incorporated. View entire conversation on ReviewNB |
Incorporated. View entire conversation on ReviewNB |
Incorporated. View entire conversation on ReviewNB |
Incorporated. View entire conversation on ReviewNB |
Incorporated. View entire conversation on ReviewNB |
I'm adding a note in the Sampler example that all the following examples will be with estimators. View entire conversation on ReviewNB |
Done. View entire conversation on ReviewNB |
Done. View entire conversation on ReviewNB |
View entire conversation on ReviewNB |
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.
Thanks a lot for the quick application of the feedback. It is looking very good! I am now going through some technical details in the content. I started reviewing on the raw notebook so that I could make suggestions on formulas, but because the image takes a lot of space in the raw file, I think I'll switch over to ReviewNB again for the remainder of this review.
Co-authored-by: Elena Peña Tapia <[email protected]>
@ElePT I have incorporated the suggested changes. Please review it at your earliest convenience. |
Thanks Fatima, my response is going to be a bit slower this week, but I will get around to reviewing before the Unitary Hack deadline :) |
@ElePT as UnitaryHack ends in about half a day, can you please review the PR so that I can claim the bounty? |
@maxwell04-wq open PRs reviewed also later in the week will be considered for unitaryHACK. |
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.
Thank you very very much for your contribution. I have left some final comments, after which, I will approve your PR. I have also confirmed you as the main candidate to obtain the unitary hack bounty :)
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.
LGTM! Thanks for your hard work :)
…) (Qiskit/qiskit-tutorials#1463) <!--⚠️ If you do not respect this template, your pull request will be closed.⚠️ Your pull request title should be short detailed and understandable for all.⚠️ If your pull request fixes an open issue, please link to the issue. ✅ I have added the tests to cover my changes. ✅ I have updated the documentation accordingly. ✅ I have read the CONTRIBUTING document. --> ### Summary Fixes Qiskit/qiskit-tutorials#1390, Unitary Hack contribution. ### Details and comments - Updates the gradients tutorial from `opflow.gradients` to `algorithms.gradients` - Use of `BaseEstimator` and `BaseSampler` classes from Primitives for gradient evaluation - Demonstration of different methods for gradient evaluation - Application example: VQE - Solved using Estimator, Sampler, and classical optimizer (scipy.minimize) - More details on SPSA gradient and Qiskit Primitives can be added --------- Co-authored-by: Elena Peña Tapia <[email protected]>
Summary
Fixes #1390
Details and comments
opflow.gradients
toalgorithms.gradients
BaseEstimator
andBaseSampler
classes from Primitives for gradient evaluation