Curve analysis with uncertainties package#551
Closed
nkanazawa1989 wants to merge 34 commits into
Closed
Conversation
f2c531b to
d9364b3
Compare
d9364b3 to
2ad7511
Compare
eggerdj
suggested changes
Dec 6, 2021
Contributor
eggerdj
left a comment
There was a problem hiding this comment.
Looks good. Minor suggestions.
Co-authored-by: Daniel J. Egger <38065505+eggerdj@users.noreply.github.com>
Co-authored-by: Daniel J. Egger <38065505+eggerdj@users.noreply.github.com>
Co-authored-by: Daniel J. Egger <38065505+eggerdj@users.noreply.github.com>
Co-authored-by: Daniel J. Egger <38065505+eggerdj@users.noreply.github.com>
…nazawa1989/qiskit-experiments into upgrade/curve_analysis_uncertainties
…ade/curve_analysis_uncertainties
Collaborator
Author
|
I also added 3-sigma interval according to https://en.wikipedia.org/wiki/68–95–99.7_rule
CI with parameter correlation nicely explains relation between sampling error and fit error. |
eggerdj
suggested changes
Dec 8, 2021
Contributor
eggerdj
left a comment
There was a problem hiding this comment.
Almost good. One minor naming concern.
…ade/curve_analysis_uncertainties
Collaborator
Author
|
Closed since this is merged into #564 for review with full functionality |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
This is followup of #481 . This PR offloads error computation currently manually coded (sometime with error) to
uncertaintiespackage.Details and comments
Usage of
FitValobject is removed from experiment library / framework module and replaced withUFloatobject. BothFitValandUFloatare numerical object consisting of the nominal value and standard error.UFloatsupports numerical operation with error propagation while keeping parameter correlation, so this will give us more precise estimate of error values that is computed based on fit values.We need minor code update for many analysis modules, because of property name difference:
However we can remove code that manually computes error propagation (without considering parameter correlation).
Effect of parameter correlation
In current Experiments curve fit code,
pcov(covariance matrix) returned fromcurve_fitis just ignored. So parameter values are (written with ufloat object in uncertainties package, but same implementation with current code)on the other hand,
uncertaintiesprovides a convenient function that generates parameter set with covariance matrix.I tested the difference with T1 experiment with not enough scan range (so that error is amplified).
Without correlation

With correlation

Note that non-diagonal part (correlation) is not negligible in such configuration, and seems like we have been overestimating the confidence interval.
(note)
This PR doesn't update data representation in
ExperimentData. This means once analysis complete the outcome is converted intoFitValanyways and we cannot compute error propagation anymore. Same for data loaded from the database. To support error propagation with loaded entries, we need to replaceFitValwithUFloatin db service module. But this change will add more and more files to review, so will be done (if we want) in follow-up PR.TODO