-
Notifications
You must be signed in to change notification settings - Fork 864
Qlbm #1270
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?
Qlbm #1270
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. |
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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 mention that there are other works on QLBMs, apart from Ref. 1.
- "practical systems requires infeasible numerical resources" - I think infeasible might be too strong here, no? may prohibitive/substantial/considerable?
- "A solution to the dynamics of the distribution function can be obtained by employing the (classical) Lattice Boltzmann Method (LBM)". This sentence is not clear enough. How about something like: The solution to this equation gives the dynamics for the distribution of particles positions and velocities. A numerical approach for solving the kinetic PDE equation can be obtained by employing the (classical) Lattice Boltzmann Method (LBM).
- The latex in the enumerated list (Streaming, Collision) seems to not render well. I see $
- f^{eq} is not defined (just say what it means).
- "Such as heat transport and magnetohydrodynamics" : I am curious, did you see those relations somewhere? actually, recently we were trying to figure out if LBM is good to magnetohydrodynamics...
- Reynolds number is not defined. I think it is OK not to write it explicitly, but sat what it means such that non-experts will be able to understand.
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.
Replies in chronological order
- Added: A variety of QBLM algorithms have been proposed (see Refs. below). Here, we focus on the collisionless case, including specular reflections. We exemplify the algorithm by analyzing an example inspired by the work of Schalkers and Moller [1].
- modified infeasible to substantial
- excellent, modified...
- not sure, show me how it renders in your notebook and I'll fix it
- added: and
$f^{\text{eq}}(\mathbf{u}(f),\rho(f),T(f))$ is the local Maxwell-Boltzmann distribution. - Yep :). I first got it from GPT, but it also appears on the wiki page, and there are papers on this topic. Also note that the quantum algorithm is used in a variety of physical cases.
- Deleted the Reynolds number in this sentence, since it's only the intro, so didn't want to go into unnecessary details, defined qualitatively (not in terms of other physical parameters, since that required defining them as well..) in the technical details (where it is mentioned next) .
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 added that the Boltzmann transport equation is in the BKG approximation
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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. I am not sure we can use it. You can do
$|\psi\rangle$ . - registers --> variables :-)
- positional and velocity variables of size log(N) and log(V), respectively. The way you wrote it sounds like there are log(N) variables.
- It will be good to remind here that you do not do collision, or maybe you do it later on. Anyway, in this paragraph you do mention collision, but you have U_ref and not U_col...so it is confusing.
- For the "Readout" mention that we must take some local, global measurement of some properties/observables. We cannot read the full statevector as then we will lose any quantum advantage.
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.
- show me what you mean when we meet (I only see a regular latex ket in your note)
- super, modified
- modified it to: are mapped to separate quantum variables. The
$N$ lattice sites and$V$ velocities are each represented by a quantum variable, composed of$\log(N)$ "positional" qubits and$\log(V)$ "velocity" qubits, respectively. not fully sure, let me know if it is still not clear - Added: There are several variants of the algorithm, depending on the specific physical scenario. For instance, reflecting objects may be absent from the medium and are therefore excluded from the evolution. In the example presented here, we consider the collisionless case, where the dynamics are governed solely by streaming and reflection.
- changed to: Readout - measurement of global observables or a restricted number of local observables. In the implemented example, we consider a small number of grid points and measure the computational basis. Didn't really want to get into this, because from my understanding it isn't quite understood how to measure efficiently something which is important (and the papers don't really go into this drawback), same goes for the preparation step. Like in HHL, the quantum advantage depends on a the possibility to efficiently load the initial state and measure something useful efficiently.
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.
"...we consider a small number of grid points and measure the computational basis" --> "and measure the full state in the computational basis"
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.
fixed
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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.
- " standard qubits" --> single qubits
- Same comment about \ket as above.
- Maybe give an example for the notation: the quantum state is |2>|4>|0>|6>|1>|4> means that in cell (2,4) there is a particle with left velocity of size 6 and down velocity of size 4?
- (descretized via CFL-based-schedule) undefined. Say it will be explained below or similar. (also a typo - discretized)
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.
- fixed
- fixed
- fixed, good catch, I even think I had something like that in the 1D version.
- modified to: Streaming: particles move one lattice site if there speed allows it (see below for further details).
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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.
Added figure captions for all the figures.
The caption of this figure is: Collisionless QLBM algorithm circuit. The preparation step and the first two iterations of the time-evolution stage.
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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 headings sizes do not fit here. The initial condition hierarchy should be the same as for the observables, no
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.
fixed it and modified the explanation of the initial conditions (it wasn't accurate)
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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,1442 @@ | |||
| { | |||
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 avoid this error?
/var/folders/f9/dqrhj23922x926lhkszh0c5h0000gn/T/ipykernel_42717/2314929453.py:29: RuntimeWarning: divide by zero encountered in divide inverse_velocities = 1/velocity_magnitudes
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.
Maybe, let's discuss this.
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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 would add a clarification:
"For example, at time step 3, distributions with absolute velocities 1,2,3 will stream", or similar.
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.
Added, the clarification
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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.
- "a constant number of ancilla qubits..." - eventually you avoid using auxiliary qubit, no?
- "Allowing multiple distinct incoming states to map onto a single outgoing state would correspond to a non-invertible transformation, thereby violating the unitarity condition required by the quantum circuit computation model." - important, cool!
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!, good catch
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.
You still mention an ancilla here:
- For particles (distributions) reaching sites within the boundaries, flag an ancilla register.
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.
rephrased this section
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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 is OK to put this code here. You can also just put the figure by hand and add the code at the end as a reference. Both options are good.
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.
I like the option of putting the code at the end.
Fixed it
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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 no ancilla here. Also, yor wrote that this function receives schedule,but in the code there is the indx parameter.
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.
I think you missed this comment, about the ancilla.
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.
your right, rephrased this paragraph
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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 #23. def reflection(qs: PhaseSpaceStruct, mag: int, limits: CArray[CReal]) -> None:
Very nice.
The naming "fixed" and "change" are a bit confusing to me, but I could not find an alternative.
A suggestion:
Maybe instead of arr: CArray[CReal] you can do arr: CArray[CArray[CReal,2],2] and then you will do something like
pos_low, pos_high = arr[0][0], arr[0][1]
limits = arr[1]
Then,
control( ((change_pos == pos_low) | (change_pos == pos_high)) & (fixed_pos >= limits[0]) & (fixed_pos <= limits[1]) & (change_u == mag), lambda: X(change_v_dir), )
Reply via ReviewNB
| @@ -0,0 +1,1442 @@ | |||
| { | |||
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 #34. x_low, x_high, y_low, y_high = limits[0], limits[1], limits[2], limits[3]
BTW, maybe it was written above, but I think that the example is restricted to a rectangular objects no?
Maybe mention in one sentence how one might generalize for arbitrary object? or equivalently a set of objects
Reply via ReviewNB
PR Description
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
.ipynband.qmodto be unique across this repository..qmod,.synthesis_options.json,.metadata.jsonIf applicable, please include link to the paper on which the notebook is based, in the notebook itself.
Please use
rebaseon your branch (no merge commits)Please link this PR to the relevant issue
Please make sure to run
pre-commitwhen commiting changesgitin the terminal, make sure to installpre-commitvia runningpip install pre-commitfollowed bypre-commit installpre-commitdocumentationpre-commit.pre-commitmay minorly alter some files. Make sure togit addthe changes done bypre-commit