Closed VarLad closed 3 years ago
@GiggleLiu Added an html and a pdf version!
Would be nice if @GiggleLiu can take a second look!
First, very nice tutorial! This is definitely gonna help the ecosystem a lot! There are some minor edition from myside.
regarding to the HTML and PDF versions, I think we should build them into a github action instead of manually upload them in the PR, because:
- it will make the git tree grow huge (so cloning this repo will be slower and slower)
- it is harder to review whether they are generated correctly
- every time someone wants to edit this tutorial will need to generate PDF and HTML manually
But I haven't got enough time to do the github action recently, so I suggest let's remove the PDF and HTML for now so we don't need to merge them into master. or maybe someone is willing to write the github action for this? I'm down to guide you.
I'm sorry, what's "github action"?Hearing about it for the first time😃 Also, is there some website, where we can just input link to Pluto notebook, and get an html as a result? As for pdfs, I think deleting all but QCTutorial would make sense!
I'm sorry, what's "github action"?Hearing about it for the first time😃
It's hard to explain in short if you haven't had a concept of CI, so reading this might help:
Also, is there some website, where we can just input link to Pluto notebook, and get an html as a result?
It should be done via Github Actions + Github Pages
As for pdfs, I think deleting all but QCTutorial would make sense!
The PDF should be auto-generated via Github Actions then upload them to release CDN. If you put them in the repo, it can be extreme slow to download in many places around the world. That's why there is a github release CDN. So let's remove all the PDFs first.
Okay.
What we can do is, merge the corrected version of Notebooks only to the repo. @Roger-luo or @GiggleLiu can make a GA, whenever they find free time. Also, if making a GA doesn't involve knowledge of JS, I can start on it immediately!
How does that sound?
If making a Github Action involves JS, I can't do it in the short timeframe.
you only need to write some shell commands in a YAML file, e.g this action runs the tests: https://github.com/QuantumBFS/YaoSym.jl/blob/master/.github/workflows/CI.yml
If making a Github Action involves JS, I can't do it in the short timeframe.
No, I'm not asking you to do it. I'm just saying let's remove HTMLs and PDFs in this PR, since this will make the git tree quite huge.
So if you want to work on a github action, let's also pending that a bit to a new PR too. I'd like to merge this PR as a Pluto notebook alone first after @GiggleLiu takes a second look.
So if you want to work on a github action, let's also pending that a bit to a new PR too. I'd like to merge this PR as a Pluto notebook alone first after @GiggleLiu takes a second look.
Thanks for the PR and sorry for the late review, the contents look good in general. I especially like the quantum arithematics section.
P5: think this notebooks can be organized better. Current style of writting is more like a documentation rather than a tutorial. Tutorials should motivate people step by step.
Some notation issues
R^x_\phi
. Why not just use {\rm Rx}(\phi)
?P6: Good! If there are some citations, it would be better. e.g. https://arxiv.org/abs/1807.02023
Also, there is probably better way to re-arrange the qubit indices.
julia> using Yao
julia> reg1 = ArrayReg(bit"111")
ArrayReg{1, Complex{Float64}, Array...}
active qubits: 3/3
julia> reg2 = ArrayReg(bit"101")
ArrayReg{1, Complex{Float64}, Array...}
active qubits: 3/3
julia> reg = join(reg1, reg2)
ArrayReg{1, Complex{Float64}, Array...}
active qubits: 6/6
julia> reorder!(reg, [1,4,2,5,3,6])
ArrayReg{1, Complex{Float64}, Array...}
active qubits: 6/6
julia> measure(reg; nshots=3)
3-element Array{BitBasis.BitStr{6,Int64},1}:
110111 ₍₂₎
110111 ₍₂₎
110111 ₍₂₎
p7: looks good, the only issue is the image display
p8 and p9: looks good. Since there are many special characters in the notebook, it would better to tell people how to type the special characters in the notebook. Same in the Grover notebook. Please add at least one citation so that user can find more detailed descriptions of the algorithms.
p10: I am not familiar with QRAMs, the only form that I know is using qutrits (https://arxiv.org/abs/0708.1879). Maybe you should mention somewhere that there is not only one definitions. A short paragraph at the begining of the notebook to introduce the background would be nice.
After fixing these issues and also Roger's issues, it should be good to merge. Thanks again
@GiggleLiu Thanks for the review!
I am a but confused by the notation of the rotation gate like R^x_\phi. Why not just use {\rm Rx}(\phi)?
A lot of books use this! Looks visually better too. Also, p5 is supposed to look like a formal explanation of the gates. Its kinda the successor of p2+p3. I don't use most of those gates in my notebooks. They just introduce the reader to a few basic concepts, while using Yao to draw some comparisons and prove/show some stuff.
P6: Good! If there are some citations, it would be better. e.g. https://arxiv.org/abs/1807.02023 Also, there is probably better way to re-arrange the qubit indices.
Not re-ordering, I've to separate the qubits! Any way to get a and b back from join(a,b)
?
Also, there's already a citation in README.md! My algorithm is based on that paper too!
p7: looks good, the only issue is the image display
Well, take a look at README.md.
Also, note that for plots to show up, you need Gaston, which is a front-end of gnuplot. So, you've to install gnuplot externally. Instructions to install gnuplot
Mac Users -brew install gnuplot
Windows users - Download and install it from here
Linux users:-
Fedora/Red-hat-based :- sudo dnf install gnuplot
Ubuntu/Debian-based :- sudo apt install gnuplot
p8 and p9: looks good. Since there are many special characters in the notebook, it would better to tell people how to type the special characters in the notebook. Same in the Grover notebook. Please add at least one citation so that user can find more detailed descriptions of the algorithms.
Regarding special characters, I copied a few from Google! Rest are from the character-list program in Linux called gnome-characters. So, yeah, it's just for the sake of appearance. The reader doesn't have to use the same characters. Regarding citations, the references I mentioned in the README.md already have some citations, along with a detailed description of the algorithm. If you still insist on adding citations, can you please provide me with some appropriate citations to add?
Also, regarding the QRAM, I learned it from IBM Quantum Challenge, and have included the same implementation and citation as that of the challenge in README.md
Ah, sorry. I didn't notice the references are in the README. Then it should be fine. Maybe you should add a new reference for the QRAM paper, because you have used a different setup.
Not re-ordering, I've to separate the qubits! Any way to get a and b back from
join(a,b)
?
To readout, one can use the APIs in BitBasis, like
julia> using Yao
julia> x = ArrayReg(bit"111000")
ArrayReg{1, Complex{Float64}, Array...}
active qubits: 6/6
julia> res = measure!(x)
111000 ₍₂₎
julia> readbit(res, 1,3,5)
000100 ₍₂₎
About the plots, I feel you should not expect a user to follow these instructions. Can we just switch to Plots? Although it is slow, it is much easier to get new users started.
Also, p5 is supposed to look like a formal explanation of the gates. Its kinda the successor of p2+p3. I don't use most of those gates in my notebooks. They just introduce the reader to a few basic concepts, while using Yao to draw some comparisons and prove/show some stuff.
What about using another name, like gateref.jl
. You can re-direct users to this notebook in other notebooks. I feel the titles of notebooks should contain more information. Like p6.jl
-> quantum_arithmetics.jl
, so that users can find contents much easier.
julia> x = ArrayReg(bit"111000") ArrayReg{1, Complex{Float64}, Array...} active qubits: 6/6
Is there any way to get an array of 6 active qubits: 1/1, from x
?
About the plots, I feel you should not expect a user to follow these instructions. Can we just switch to Plots? Although it is slow, it is much easier to get new users started.
I implemented it in Plots first, and then CairoMakie... No one wants to wait that long for the plots to show up, in just tutorials.... And think about the ones with much older devices.... I see two alternatives to this!
I root for the above solution 1, as its easier if I want to implement something new in future tutorials. Also, that makes the GA and HTML version all the more important!
julia> x = ArrayReg(bit"111000") ArrayReg{1, Complex{Float64}, Array...} active qubits: 6/6
Is there any way to get an array of 6 active qubits: 1/1, from
x
?
I do not understand your question, how does the machine know the state is a product state? And is there any issue using measure?
About plotting, I'd rather wait for 7.3s. In upcoming Julia 1.6, this time will be much smaller.
(base) ➜ Pluto git:(pr14) ✗ julia
_
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.5.2 (2020-09-23)
_/ |\__'_|_|_|\__'_| | Official https://julialang.org/ release
|__/ |
julia> @time using Plots
7.333277 seconds (17.37 M allocations: 1.001 GiB, 4.90% gc time)
If you use Yao and YaoPlots first, it is 4s.
(base) ➜ Pluto git:(pr14) ✗ julia
_
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.5.2 (2020-09-23)
_/ |\__'_|_|_|\__'_| | Official https://julialang.org/ release
|__/ |
julia> using Yao, YaoPlots
julia> @time using Plots
4.031252 seconds (9.21 M allocations: 555.635 MiB, 2.09% gc time)
julia> reorder!(reg, [1,4,2,5,3,6]) ArrayReg{1, Complex{Float64}, Array...} active qubits: 6/6
Okay! I'm really sorry! I'm understanding what you were saying in the first place! And yeah, it can be done!
Edit: Makie doesn't get a speed boost in 1.6beta... which was honestly a letdown. Also, Plots has become a lot faster😀 I'm going ahead with plots, with gr() by default and inspectdr() as a recommendation if someone finds gr() uncomfortable.
@GiggleLiu
What about using another name, like gateref.jl. You can re-direct users to this notebook in other notebooks. I feel the titles of notebooks should contain more information. Like p6.jl -> quantum_arithmetics.jl, so that users can find contents much easier.
Well.... I'd love to do that... but don't know how to.... Where do I link to? The github page? raw version? http://localhost:1234/open?url=abc.xyz?
Also, I've noticed that Jupyter notebooks get rendered in Gtihub! Any way we can get the same done for Pluto? If yes, it'd be great to just link to the github page Another option is when you've made a GA for html and pdf generation, then link to the html version using link
@GiggleLiu New plotting format! Please take a look! I already think the new one is way better than what Qiskit provides.😀
Also, I've noticed that Jupyter notebooks get rendered in Gtihub! Any way we can get the same done for Pluto? If yes, it'd be great to just link to the github page
well for this we need to set up a github action to export HTMLs automatically first
This plot looks wield. Is this just me?
@GiggleLiu It certainly does!😂 I remember that my biggest problem with Qiskit histogram was, for any more than 4-6 qubits, the histogram was unreadable...
So I thought increasing the width would be a good idea, so that we've a scroll bar!Then GR makes weird problems with axis and tics when I do that!Also, axis of InspectDR is outright broken for 1.6beta!So unable to find a solution, I made them disappear. Somebody recommended annotations and they did a great job, when I implemented them!
I think its new, and maybe weird, but achieves its job successfully!
so maybe can we merge this first if it looks good in general, then @clad26 can work on some PRs to further polish it - I will need to use what's in this PR to test against the github actions and new websites etc. what do you think? @GiggleLiu
@Roger-luo
so maybe can we merge this first if it looks good in general, then @clad26 can work on some PRs to further polish it
As I mentioned above, the problem is on the GR/Pluto/InspectDR side, so I won't promise a change unless it gets fixed! Without having proper tics in InspectDR (and no tics in GR in Pluto for wider plots), this is as good as it gets, I guess🙂
If anyone has better idea for plots, they're more than welcome😄
As I mentioned above, the problem is on the GR/Pluto/InspectDR side, so I won't promise a change unless it gets fixed! Without having proper tics in InspectDR (and no tics in GR in Pluto for wider plots), this is as good as it gets, I guess🙂
no worries, I think I just need something to test against first, we can figure out some solution together in the future on this problem. I think the first thing is to get the new tutorial pipeline running.
@GiggleLiu Any other changes you'd like to recommend?
@GiggleLiu if not, I think it might be ready to merge😃
Yeah, let’s leave the plotting issue for now
Well!The problem is, the plots look like crap.😶 The histogram itself displays the data it needs to, perfectly!
Also, it appears that folks at Makie are working to reduce first time to plot😃
New notebooks, formatting choices and pdfs