-
Notifications
You must be signed in to change notification settings - Fork 42
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
Utility scale QAOA tutorial #1238
base: main
Are you sure you want to change the base?
Conversation
Thanks for contributing to Qiskit documentation! Before your PR can be merged, it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. Thanks! 🙌 |
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Happy to do style/language review when this is ready to go. Just lmk! |
posting here some comments received through other channels:
|
View / edit / reply to this conversation on ReviewNB javabster commented on 2024-05-02T20:53:17Z I think we can remove the first cell with the authors and the patterns, it feels a bit out of place with the style of all the other tutorials |
View / edit / reply to this conversation on ReviewNB javabster commented on 2024-05-02T20:53:18Z I think these cells with the call to action and team 1 or 2 should be removed, I guess that was part of an interactive wrokshop but it doesnt really work in tutotiral format |
View / edit / reply to this conversation on ReviewNB javabster commented on 2024-05-02T20:53:18Z also these call to action sections with the commented out code should probably be uncommented i guess? |
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.
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.
Are all these .qpy files necessary? Users should be able to download and run the notebooks, so any additional data and utility functions should be put into the notebook itself. afaik we dont have the ability on the learning platform to handle additional data files like this
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.
Ok, I have deleted all files except the experiment results. If the LP doesn't support extra data files we may remove it as a final step to avoid losing the plots
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 we come up with a more descriptive title for this tutorial? It should be something specific (and ideally enticing) for a reader to want to click on it
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 you check that the headings for the pattern steps match what is used for other tutorials?
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.
In general I think this notebook could use a grammar and spelling check (cc @abbycross 😄 )
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 think it would be nice to call out in the intro section of this tutorial that you can run it within the 10 minute limit of the open plan (also make sure to check that is actually true, I haven't personally validated that claim 😅 )
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.
For the introduction to step 2 when you're giving the TL;DR about transpilation I think it would be good to direct people towards some of the docs.quantum.ibm.com/tranpile resources for more detail
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 we come up with a more descriptive title for this tutorial? It should be something specific (and ideally enticing) for a reader to want to click on it
I'm giving another stab but maybe someone with more imagination can help me?
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.
Also in the step 2 intro - why is a simulator used? we should always show a real device unless absolutely necessary to do otherwise. Also I'm generally not really sure what the purpose of that code snippet is for, if it is just to show what transpilation is I think we can cut it and instead add some text encouraging people to go to the docs if they dont know what transpilation is
- Simulator was use for ease of usage, we can definitely switch to hardware.
- We need to transpile the circuit to the hardware before execution (the sampler/estimator don't do that anymore). Step 2 seems the most natural place to put the transpilation step
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.
in scale up step 3 I think we should remove the explanation about loading saved data and only show the code cells that show users how to do it themselves. Mostly because we dont have a way in LP to load saved data like this
Ok, let's do this as a last step to avoid having to keep sending jobs to the device to execute the notebook
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.
in the scale up section the first diagram of the nodes looks janky 😅
I need some help with this, have very little idea of plotting with rustworkx
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.
in the scale up section the first diagram of the nodes looks janky 😅
I need some help with this, have very little idea of plotting with rustworkx
It doesn't seem like there's much control over it. If there's some way to get the coordinates they use for the graphs on quantum.ibm.com then we can input them into the drawer, but I imagine that'll convolute the tutorial.
I don't think it's too bad at the moment so might be best leaving it.
@miamico could you also please replace the E.g. for the first example: import rustworkx as rx
from rustworkx.visualization import mpl_draw as draw_graph
import numpy as np
n = 5
graph = rx.PyGraph()
graph.add_nodes_from(np.arange(0, n, 1))
edge_list = [(0, 1, 1.0), (0, 2, 1.0), (0, 4, 1.0), (1, 2, 1.0), (2, 3, 1.0), (3, 4, 1.0)]
graph.add_edges_from(edge_list)
draw_graph(graph, node_size=600, with_labels=True) |
Github won't let me suggest edits because of the size of the notebook file, so I am making some IBM style updates (such as rewording not to use "we") locally, and will merge to this branch as soon as I've gotten through it! |
Looking at Pattern step 2 in the text: we should add a link to the docs for transpiler passes. Is this the right link to add? https://docs.quantum.ibm.com/api/qiskit/transpiler_passes#transpiler-passes -- or instead, something in the platform docs (https://docs.quantum.ibm.com/transpile)? cc: @kaelynj |
I am hesitant to "IBM-ify" the style of the Appendices, but also wonder if they belong at the end of the notebook since we don't use appendices anywhere else. Is there a better place to store and display that content? Thoughts @javabster @kaelynj @miamico ? |
I would vote for having appendices as this seems to be a common occurence in the utility notebook I've been working on. Maybe a link to external reference would be enough though |
Adds a new tutorial for QAOA with code for running experiment at utility scale. All dependencies from Application modules are removed