-
Notifications
You must be signed in to change notification settings - Fork 70
Update the docs to showcase the new backend interface #952
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: master
Are you sure you want to change the base?
Conversation
* Fix default qutip options * unlock pipeline * Fix unlocking pipeline * Fix mypy
Bring in changes from v1.6.2
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -1,510 +1,610 @@ | |||
| { | |||
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 rather say QutipBackendV2 and should not reference QutipEmulator anymore.
Also, see my comment below on the relevance of job_params
Reply via ReviewNB
|
Not directly related to this MR but I have a question that bugs my mind: why do we have two separate backend for remote and local emu_mps (and similarly for qutip)? Could we not get away with a single backend and rely on the connection to know whether the execution is remote or local? |
2d4fc49 to
6751bbf
Compare
We could, but we want these backends to be usable without needing the extensions for local execution (i.e. |
Yeah I assumed the dependency for the local execution was the motivation behind it. Anyway what we have works, we'll see if users complain about it, no need for any action for now |
HGSilveri
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.
This is a substantial improvement over the previous iteration, nice job!
tutorials/advanced_features/Backends for Sequence Execution.ipynb
Outdated
Show resolved
Hide resolved
tutorials/advanced_features/Backends for Sequence Execution.ipynb
Outdated
Show resolved
Hide resolved
tutorials/advanced_features/Backends for Sequence Execution.ipynb
Outdated
Show resolved
Hide resolved
tutorials/advanced_features/Backends for Sequence Execution.ipynb
Outdated
Show resolved
Hide resolved
tutorials/advanced_features/Backends for Sequence Execution.ipynb
Outdated
Show resolved
Hide resolved
tutorials/advanced_features/Backends for Sequence Execution.ipynb
Outdated
Show resolved
Hide resolved
|
Since this PR relies on behavior that was only fixed in |
a07f857 to
4462f2a
Compare
| @@ -0,0 +1,633 @@ | |||
| { | |||
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.
- Should we change the title to indicate this is focused on emulators?
- Last sentence before
1. Creating ...has some typos
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.
Based on your suggestion to change back the file name and append the discussion about changes on the QPU, then yes, let's keep the old title. I've also appended a mention to this new section in the what you will learn.
| @@ -0,0 +1,633 @@ | |||
| { | |||
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 highlight the last sentence in a callout box because I feel like it's a common gotcha
Reply via ReviewNB
| @@ -0,0 +1,633 @@ | |||
| { | |||
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.
- Let's maybe also show the standard way of getting the result for a specific time
- nit: These numpy types are a bit weird and unnecessary, I wonder if we can easily get rid of them
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.
Since with a default config, only results at the final time are returned I cannot showcase it here. In the next section, with a custom config, I have added the fidelity halfway through the sequence, and I will demonstrate how it works.
| @@ -0,0 +1,633 @@ | |||
| { | |||
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.
nit: I think using the Fidelity as a means to get the probability of measuring a computational basis state is perhaps not the most enticing example (because the information could easily be extracted the Counter). Measuring the fidelity to some superposition state would be much more interesting imo
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.
Using the bell state instead.
| @@ -0,0 +1,633 @@ | |||
| { | |||
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.
How about having more than one evaluation time, so that the user can see how that works?
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.
indeed, see my comment on your earlier suggestion in this direction.
| @@ -0,0 +1,633 @@ | |||
| { | |||
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.
nit: The base class is actually RemoteConnection but I don't think we want to go into those details
Reply via ReviewNB
| @@ -0,0 +1,633 @@ | |||
| { | |||
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 whole part about the devices is not true for emulators, we should still be able to use AnalogDevice. Did you try and fail?
Reply via ReviewNB
| @@ -0,0 +1,633 @@ | |||
| { | |||
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 you sure about this? Did you try just not giving the job_params? I should work without them but I might be missing something
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.
PasqalCloud requires this. You also cannot just feed it the empty dict
| @@ -0,0 +1,633 @@ | |||
| { | |||
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.
Even if runs>1 you would have gotten just one. You would have had to have given multiple entries in job_params to get multiple Results
Reply via ReviewNB
| @@ -0,0 +1,633 @@ | |||
| { | |||
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.
Do you think it's valuable to go into these technical details here? Would perhaps a link to Emu-mps docs be better?
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.
indeed
|
Also, can you make the PR target |
| "Emulator backends also accept, optionally, a configuration given as an instance of the `EmulationConfig` class. As can be seen from the [docs](../apidoc/_autosummary/pulser.backend.EmulationConfig.html#pulser.backend.EmulationConfig), this class defines a few generally applicable configuration options, and then accepts backend specific options as keyword arguments. Each backend defines a subclass of `EmulationConfig`, such as `QutipConfig` for `QutipBackendV2`, that makes explicit the specific options for that backend. Since we are not interested in changing configuration options specific to `QutipBackendV2`, we use the `EmulationConfig` class directly.\n", | ||
| "\n", | ||
| "To showcase the flexibility of emulation backends specifically, we will configure `QutipBackendV2` to return the probability, at the final time, of measuring the bitstring `11`, which can be done using the `Fidelity` observable documented [here](../apidoc/_autosummary/pulser.backend.Fidelity.html#pulser.backend.Fidelity). \n", | ||
| "To showcase the flexibility of emulation backends specifically, we will configure `QutipBackendV2` to return the probability, halfway through the sequence and at the end, of being in the bell state, which can be done using the `Fidelity` observable documented [here](../apidoc/_autosummary/pulser.backend.Fidelity.html#pulser.backend.Fidelity). \n", |
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.
Instead of "probability to be in the bell state", I think "overlap with the bell state" would be more accurate
| "metadata": {}, | ||
| "source": [ | ||
| "We can see that there is only one `Results` object in the list after the job is done since `runs=1`. Note that since we ran the same sequence, as locally, the fidelity will be the same as in that case." | ||
| "We can see that there is only one `Results` object in the list after the job is done since only a single item was present in `job_params` above. Note that since we ran the same sequence as locally, the fidelity will is the same as in that case." |
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 time we specify only one evaluation_time, do we maybe want to replicate the local example exactly?
Also
| "We can see that there is only one `Results` object in the list after the job is done since only a single item was present in `job_params` above. Note that since we ran the same sequence as locally, the fidelity will is the same as in that case." | |
| "We can see that there is only one `Results` object in the list after the job is done since only a single item was present in `job_params` above. Note that since we ran the same sequence as locally, the fidelity is the same as in that case." |
The significant change is to
Backends for Sequence Execution.ipynbwhere I'm using the newEmuMPSBackendandEmuFreeBackendV2, but I also removed some unneeded imports in another notebook.