-
Notifications
You must be signed in to change notification settings - Fork 865
Shadows #966
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?
Shadows #966
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
@@ -0,0 +1,512 @@ | |||
{ |
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 @gzemlevskiy17 for this PR! Please provide details in Markdown about the code so we can review it properly.
- Short introduction about what is implemented.
- Explain major code blocks, and how they are related to parts of the implementation.
- Add a snapshot of the quantum part from Classiq visualization.
Reply via ReviewNB
@@ -0,0 +1,512 @@ | |||
{ |
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.
If you like, we have a function in our open-library that prepares a bell state. You can use it as well if you want.
Reply via ReviewNB
@gzemlevskiy17 are you still working on this? |
@TomerGoldfriend Yes, sorry I was working on developing the Classiq challenge for the FLIQ hackathon. I will update this PR soon. |
Hi @gzemlevskiy17 ! |
Hi @NadavClassiq. Yes! The project is finished, but I am still figuring out how to adhere to the contributing guidelines. I actually have a few questions: I ran pre-commit and one test did not pass, specifically:
What should I do about this? Also, since classical shadows involves applying random unitary gates, the circuit, and thus the quantum model, vary, so I don't know if I should generate a .qmod file as per the guidelines. Finally, I am unsure of the git workflow I should follow, particularly where rebase comes in. |
Hi @gzemlevskiy17!
Lmk if you need further help :) |
@NadavClassiq Thank you! If I am contributing to community/, what would I write for the |
Hi @gzemlevskiy17 ! You can use a shorter name, just write the full name with a proper link in the notebook. Lmk if you need anything else :) |
@NadavClassiq I've pushed my latest changes. One part of my code uses a list of PauliTerms as a Hamiltonian, but since that has been deprecated as input for |
Great Job @gzemlevskiy17 ! I'll be reviewing it soon, maybe with feedback and suggestions for improvement. Thanks for the efforts! |
@@ -0,0 +1,893 @@ | |||
{ |
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.
@@ -0,0 +1,893 @@ | |||
{ |
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.
The \ket did not render. Maybe there is a way to do \usepackage{braket} in markdown. Otherwise, use \rangle and \langle, which should be fine.
"tomographically complete ensemble": this is not defined. You do not need to provide a full and rigorous definition. Just mention that you will provide one example below.
"Select a random unitary transformation from a predefined, tomographically complete ensemble" -->
"Select a random unitary,
Reply via ReviewNB
@@ -0,0 +1,893 @@ | |||
{ |
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.
@@ -0,0 +1,893 @@ | |||
{ |
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.
Maybe choose a different example than the one in pennylane? How about taking a state where you can also see some phase difference? for example \Phi^{-} where you will see a (-) sign.
Reply via ReviewNB
@@ -0,0 +1,893 @@ | |||
{ |
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.
@@ -0,0 +1,893 @@ | |||
{ |
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.
Line #7. in_snapshots (list): List of snapshots. Will most likely be the global snapshots
(list of ints in {0, 1}).
I do not understand the "Will most likely be..." comments here.
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.
If the user changes the implementation, then it might be different, but I will remove that phrase to avoid confusion.
@@ -0,0 +1,893 @@ | |||
{ |
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 you can find some numpy or scipy function for this, rather then writing it by yourself. It will be clearer, in my opinion.
Reply via ReviewNB
@@ -0,0 +1,893 @@ | |||
{ |
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.
Work with the new SparsePauliOp
instead of list[PauliTerm]
. You can do observable = Pauli.Z(0)*Pauli.Z(1)
. This will work when calling es.estimate()
. However, you will have to modify your estimate_observable
.
Reply via ReviewNB
@@ -0,0 +1,893 @@ | |||
{ |
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.
Line #1. def error_bound(error, observables, failure_rate=0.01):
This function depends on the choice of the unitary ensemble, no?
If yes, please mention it.
Reply via ReviewNB
@@ -0,0 +1,893 @@ | |||
{ |
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.
Please remove this reference. It is OK that this notebook is inspired by that reference, however, please make sure that you do not copy text, and do not use the exact code and naming.
Reply via ReviewNB
@gzemlevskiy17 any updates on this? |
@TomerGoldfriend Thank you for the suggestions! Sorry for the delay, I have had a busy start to the semester. I will be working on it this weekend. |
@TomerGoldfriend I replied to some of your comments on ReviewNB, could you please take a look? |
Sure @gzemlevskiy17 , thanks for the update. |
@TomerGoldfriend Were you able to take a look at my questions? If not, please advise how to proceed, thanks! |
@gzemlevskiy17 not yet, I will review until the end of this week. |
@gzemlevskiy17 I think I answered all your questions, please let me know if otherwise. |
PR Description
Implement a demo of the classical shadows algorithm.
I was unsure on how to use rebase or pre-commit.
Some notes
Please make sure that the notebook runs successfully with the latest Classiq version.
Please make sure that you placed the files in an appropriate folder
.ipynb
and.qmod
to be unique across this repository..qmod
,.synthesis_options.json
,.metadata.json
If applicable, please include link to the paper on which the notebook is based, in the notebook itself.
Please use
rebase
on your branch (no merge commits)Please link this PR to the relevant issue
Please make sure to run
pre-commit
when commiting changesgit
in the terminal, make sure to installpre-commit
via runningpip install pre-commit
followed bypre-commit install
pre-commit
documentationpre-commit
.pre-commit
may minorly alter some files. Make sure togit add
the changes done bypre-commit