-
Notifications
You must be signed in to change notification settings - Fork 134
Update computation of probability #424
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
Changes from all commits
8414ada
26b5b82
7a62da0
482378c
fa3537a
0f7a504
4c66e3a
1ddc994
e225ff1
3211c4d
3f64364
1e8750f
cd7238a
d05a3b0
f9a664a
10b00a0
ac5b44a
a6570a6
bcb62c5
b74c942
b85c159
eaa97de
a9a43c1
5484f99
b95d833
e534cea
c2dfbbe
3f61e3f
7e0efec
4290bc4
aca3d6b
f28c924
4ee4383
62f7d82
194568b
08efb3d
11f6c58
947f952
b7c6c06
5351a81
1ceee7e
7c8b142
8c12a8d
93276b7
9f3e946
da4bf5d
9fe8770
8291aa8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,8 +22,8 @@ | |
| from qiskit.quantum_info import Clifford | ||
| from qiskit.circuit import Gate | ||
|
|
||
| import qiskit_experiments.data_processing as dp | ||
| from qiskit_experiments.framework import BaseExperiment, ParallelExperiment, Options | ||
| from qiskit_experiments.curve_analysis.data_processing import probability | ||
| from .rb_analysis import RBAnalysis | ||
| from .clifford_utils import CliffordUtils | ||
| from .rb_utils import RBUtils | ||
|
|
@@ -88,7 +88,12 @@ def __init__( | |
|
|
||
| # Set configurable options | ||
| self.set_experiment_options(lengths=list(lengths), num_samples=num_samples) | ||
| self.set_analysis_options(data_processor=probability(outcome="0" * self.num_qubits)) | ||
| self.set_analysis_options( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we use the get function of the data processor library?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked but we cannot use the getter. This is a custom processor for RB, because outcome depends on the number of qubits, and it counts for label "0" instead of "1". Of course we can modify the getter, but this should be done in the separate PR. |
||
| data_processor=dp.DataProcessor( | ||
| input_key="counts", | ||
| data_actions=[dp.Probability(outcome="0" * self.num_qubits)], | ||
| ) | ||
| ) | ||
|
|
||
| # Set fixed options | ||
| self._full_sampling = full_sampling | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| --- | ||
| fixes: | ||
| - | | ||
| Update the :class:`~qiskit_experiments.data_processing.Probability` | ||
| data processing node to compute the estimated mean and standard deviation | ||
| of a measurement outcome probability using a Bayesian update of a | ||
| a Beta distribution prior given the observed measurement outcomes. | ||
|
|
||
| The default uninformative prior assumes ignorance about the probability | ||
| to be estimated and will prevent the estimated mean from being exactly | ||
| 0 or 1, and prevent the estimated standard deviation from being 0, which | ||
| could cause issues with computing weights during curve fitting. A custom | ||
| prior can also be provided if prior information about the probability is | ||
| know. |
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 prompted this change?
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.
Previously, I let the FFT module estimate a frequency regardless of the signal amplitude. In some test cases, especially when the signal is really weak, the guess module picks random frequency (because computed probability has been changed), and it hurts frequency guess of averaged oscillation frequency measured in x, y, z basis. So I decided to ignore the guess when SNR is likely low, i.e. it may pick some artifact.
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 it! I wonder if using the actual SNR (min-max)/(sqrt(variance)) might be nice in case someone tries to fit a very small/off resonant rotation with this at some point, so that they can average a lot and still do it? Might be a separate PR
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.
That is nice idea. What is the variance here? Is it the variance of y values
np.std(data.y)or one computed from the sampling errordata.y_err? If latter one, probablynp.mean(data.y_err).