-
Notifications
You must be signed in to change notification settings - Fork 871
Block encoding of select structured matrices #1309
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
base: main
Are you sure you want to change the base?
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
🔥 New notebook just dropped! @amir-naveh , @TomerGoldfriend — come check out this shiny new addition to our repo. |
|
Thank you @virajd98 for this contribution. We will review it soon. |
|
Hi @TomerGoldfriend I also added another notebook in the same PR based on issue # 1267. No changes have been made to the previous notebook based on issue # 1268. |
74362d1 to
3f37d93
Compare
3f37d93 to
faadc50
Compare
|
Hi @TomerGoldfriend, any updates on the reviews for this pull request? |
|
Hi @virajd98 , this is under review now. |
roie-d-classiq
left a comment
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.
Hi Viraj, excellent contribution! We have started reviewing the notebooks and hope to complete the review sometime next week.
| @@ -0,0 +1,1374 @@ | |||
| { | |||
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.
Excellent, very clear and concise!
I recommend:
- Adding a number of examples of applications, and stating them after the sentence "many applications, especially ones involving problems of linear algebra" , for example..., this helps convey the point that this is a useful algorithm.
- I would include the "Notebook contents" in the paragph before that, in addition, maybe explain the main point of the algorithm (alternative labelling, which leads to an efficient data loading for certain matrices, following the alternative labelling is mapped by arithmetic function to the indicies of the matrix) in that paragraph as well, that should make the main idea of the algorithm already clear from the start.
- possibly, add keywords instead of "notebook contents".
Reply via 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, all these points are now addressed in the latest update.
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.
Hey, excellent.
Another point is that it is worth mentioning in a sentence what the notebook contains, e.g., we consider examples of...
| @@ -0,0 +1,1374 @@ | |||
| { | |||
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.
- Define d and A_d, something like, {A_d} are the corresponding data values, where d indexes the distinct data values.
- ״Note: If the above equality does not hold, we pad״, just out of curiosity, do they give a specific algorithm for this procedure? It doesn't sound so trivial to me to add zeros while keeping the relation above the note satisfied...
- possibly missing $ in (d,m) (maybe it is only in the review mode) double check that the equation turns out fine in latex.
- the effect of O_data is ... - the wording sounds weird to me, rephrase, possibly "the operation O_data loads teh correct values:"
- dataloading --> data loading
- in the important note max --> \text{max}
- didn't quite understand the relation of O_data and the circuit, worth explaining a bit where O_data enters the circuit and saying something about the circuit components. If this is a complicated task, maybe explain this in a high level and say details are given in the paper (with the reference).
Reply via 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.
Addressed. The paper doesn't have a specific algorithm for padding with zeroes. Based on my understanding chosing any one variable for padding and padding till equality is satisfied will work.
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.
- still not so clear what the relation is of the circuit diagram components to the theory described, the different components of the circuit should be described and thier connection to the theory. If this is very complicated I wouldn't show the circuit, as it might confuse the reader.
| @@ -0,0 +1,1374 @@ | |||
| { | |||
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.
- helper --> Helper function
- Explain what is the reduced state vector you are referring to, why the comparison verifies the correctness of the algorithm.
Reply via 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.
Done.
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!
| @@ -0,0 +1,1374 @@ | |||
| { | |||
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.
Before considering the examples, maybe have a headline saying Examples, and give a short overview of what the examples analyzed are and maybe mention in which problems these matrices arise.
Reply via 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.
Done.
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.
Excellent description!
| @@ -0,0 +1,1374 @@ | |||
| { | |||
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.
- From my calculation, the formula only works if mid is either 0 or 1, take for instance m_high, m_mid, m_low = N/2 -1, we get m = N*(N/2 -1) + (N/2)*(N/2-1) +(N/2-1) = N^2*(3/4) -N -1 not N^2/2 - 1.
- what is the definitions of m_high, m_mid, m_low?
- Define O_c and O_r (possibly already in the previous circuit)
- possibly, to make the mapping clear maybe add a very simple example, demonstrating the mapping.
Reply via 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.
Correct, m_mid \in {0,1}. I provided the definitions for m_high, m_mid, m_low and an example to demonstrate the mapping. This should hopefully clear things up.
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.
Excellent!
| @@ -0,0 +1,1374 @@ | |||
| { | |||
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.
- Shortly explain what the functions array_swap, and checkerboard_BE do, and their inputs and outputs,
- Explain the results, what are s,l and data variables, what should the comparison of the state vectors convey?
- Here you get a minus sign difference between the theoretical and evaluated result, why?
Reply via 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.
Done. Statevector results are match upto a global phase. So the results match upto an overall sign in the notebook. I stated this explicitly.
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.
Super!
| @@ -0,0 +1,1374 @@ | |||
| { | |||
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.
- what is done with out-of-range pairs? worth explaining this.
- period at the end of i,j,m line
- any column has --> any column have
- after data label d" finish sentence and start "Hence, "
- as their range also match --> as their range match
- the circuit parts and the connection to the code isn't so clear, required some explaining
Reply via 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.
Done. More explanations included.
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.
super
| @@ -0,0 +1,1374 @@ | |||
| { | |||
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.
recommendation:
- Add a concluding sentence explaining the results, or just highlighting the agree
Reply via 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.
Done.
| @@ -0,0 +1,1374 @@ | |||
| { | |||
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.
Note, that this is a only one possible approximation of the laplacian (second order taylor expansion), higher order approximations exist, giving better result on account of more elements in the matrix.
Reply via 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.
Yes, the paper uses a second-order finite difference approximation. Higher order approximations would give a less sparse matrix and the structure would deviate from block-pentadiagonal. The paper doesn't talk about block encoding such structures but this would be something nice to investigate in the future.
| @@ -0,0 +1,1374 @@ | |||
| { | |||
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 is a difference in the minus sign between the two results, why? I think this arises from the normalization by a global phase, which can be fixed by modifying the project state vector function a bit.
does this difference matter?
General comment:
The notebook is really good with a lot of examples and excellen explanations:)
For me, I am missing a bit of an explanation of the results, why you compare the reduced vectors, what does that imply and a bit of a general overview in the beginning mentioning what are the examples you will consider...
good job!
Please put the notebook in the community folder for now, we will maybe later move it to applications.
Thanks a lot!!
Reply via 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.
Thank you for reviewing. I have addressed all the comments. Also moved the notebook to the community folder.
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.
Great work!, I'll approve you to merge, you can consider my recommendations and merge.
Thanks for the contribution!
|
Hi @virajd98, thanks for the submission, good job. I reviewed the notebook and left some comments, please let me know if something is unclear. |
Hi @roie-d-classiq , thank you. I'm working on the comments and will update the PR within the next 2 days. This PR has a total of 2 notebooks, I'm not able to see any comments on the second one. Did you get a chance to review that? |
| @@ -0,0 +1,791 @@ | |||
| { | |||
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.
Good job Viraj!
A few remarks:
- I’ve left comments in the specific places where grammatical corrections are needed.
- It would be instructive to include an example showing how an integer index j looks when represented as a 2^n-dimensional vector.
- regarding the implementation: kudos on the work. I would suggest rewriting it using Qmod’s LCU function, or alternatively, the prepare–select function, which can be tailored to the specific structure you used (see https://docs.classiq.io/latest/qmod-reference/api-reference/functions/open_library/linear_combination_of_unitaries/).
- Please change the location of the files to the Community folder.
- If there is any general remark about the second notebook that also applies to this one, please address it here as well.
Reply via 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.
Thank you. Done.
| @@ -0,0 +1,791 @@ | |||
| { | |||
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.
- Generally, there shouldn't be spaces between words that are connected with a "-"
- "An m-qubit unitary..."
- "provides
forefficient quantum circuits for block encoding dimensional Laplacians" - "involving linear algebra problems"
- "it is (it's) possible to efficiently..."
- "Block encoding circuits for D-dimensional Laplacian matrix with periodic boundary conditions along all dimensions."
Reply via 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.
Done.
| @@ -0,0 +1,791 @@ | |||
| { | |||
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.
- When discussing the periodic boundary conditions, v is not defined before the equation (I assume it is a general notation for a function). I recommend writing it explicitly in the text and changing the notation to u so that it is consistent with the later notation.
- It would be instructive to include an example showing how an integer index j looks when represented as a 2^n-dimensional vector.
Reply via 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.
Done.
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.
This is great.
| @@ -0,0 +1,791 @@ | |||
| { | |||
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.
Kudos on the work!
I would suggest rewriting it using Qmod’s LCU function, or alternatively, the prepare–select function, which can be tailored to the specific structure you used (see https://docs.classiq.io/latest/qmod-reference/api-reference/functions/open_library/linear_combination_of_unitaries/).
You could also compare and benchmark the two methods.
Reply via 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.
Except for the rewriting it as a LCU, I have addressed all the comments.
I wasn't sure of the comment: When you say rewrite, do you refer to using the same select logic but use prepare–select function of Classiq? If this is the case then shouldn't both the implementations be exactly the same in which case there is nothing to compare it against? Or are you referring to taking the matrix and forming an lcu in the Pauli basis, use the traditional lcu_pauli function of classiq and then compare it against the method suggested in the paper?
Regarding option 1, I'm facing some issues primarily because the 'select' argument of the prepare–select function takes only the block argument or in other words the registers with the controls. So my implementation wasn't working, surely I'm missing something which I can try post Jan 12 as I would be on holidays. If this is not absolutely necessary for the notebook, please accept this version and I'll submit a new PR once I find the solution for this. Thank you for your review.
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'm sorry for not being clear, I meant to use the prepare-select function/ the LCU Built-in Qmod function and (optionally) benchmark the produced circuits (the new method vs your first implementation) in terms of width & depth.
I will accept this version and you can resubmit after the holidays
Happy holidays!!
| @@ -0,0 +1,791 @@ | |||
| { | |||
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.
Kudos on the work!
I would suggest rewriting it using Qmod’s LCU function, or alternatively, the prepare–select function, which can be tailored to the specific structure you used (see https://docs.classiq.io/latest/qmod-reference/api-reference/functions/open_library/linear_combination_of_unitaries/).
You could also compare and benchmark the two methods.
Reply via ReviewNB
|
Hi @virajd98, yes @Adamg-classiq has reviewed your notebook, let me know if you don't see the comments. |
|
Hi @roie-d-classiq and @Adamg-classiq Thank you. I have addressed all the comments except one. If any more major changes are needed I can do only after Jan 12. Please let me know. |
|
Hi @virajd98 I will accept this version, and you can resubmit a corrected notebook after the holidays. Happy holidays!! P.S.- |
|
Hi @virajd98, Thanks for the contribution! |
PR Description
This PR implements has two notebooks and implements key components:
Deliverable includes two detailed Jupyter notebooks containing:
Circuit implementations of block encoding of certain structured matrices.
Comparison of results from circuit simulations with classical calculations.