Update ExperimentData to store figures #41

chriseclectic · 2021-05-06T23:43:23Z

Summary

Updates the ExperimentData class to handle storing figures and to bring its API a bit closer in line with the draft API of the terra base class in Qiskit/qiskit#5499

This is intended as step towards being able to make ExperimentData a subclass of the ExperimentDataV1 class from Qiskit/qiskit#5499 once it is merged into terra.

Details and comments

Changes figure return type of _run_analysis to be a pyplot.figure rather than an axis. This allows the figures to be stored and retrieved from experiment data.
Ports implementation of add_figure and figure from Experiment data and service Qiskit/qiskit#5499
Adds curve fit plot to T1Experiment
Changes ExperimentData.data from property to a method to match terra class API
Adds analysis kwarg to BaseExperiment.run which can be set to false to prevent analysis from being run automatically.
Clean up composite analysis code

yaelbh · 2021-05-09T08:31:29Z

qiskit_experiments/experiment_data.py

+        """
+        return self._backend

+    def add_data(


I see that you copied a lot of sections from the Terra PR. I assume that this is a temporary copy, and that once the Terra PR is merged and we inherit from the class there, these sections will be removed.

Yep exactly

I have a follow up branch to this PR where I started to do this, but its needs a few small changes to the Terra PR that I left as review there: https://github.com/chriseclectic/qiskit-experiments/blob/resultdb/qiskit_experiments/experiment_data.py#L34

Please be aware of #29

nkanazawa1989

This looks good to me. My comments are just questions.

qiskit_experiments/base_experiment.py

nkanazawa1989 · 2021-05-12T03:56:54Z

qiskit_experiments/characterization/t1_experiment.py

+            self._format_plot(ax, fit_result, qubit=qubit)
+            figures = [ax.get_figure()]
+        else:
+            figures = None


Raising UserWarning would be more friendly :)

qiskit_experiments/experiment_data.py

Also makes some changes to be closer to draft API for terra base class

* wip curve fit analysis * wip curve fit analysis * curve_fit complete * - unittest - bug fix - lint * black * removed redundant code * fix unittest * fix docstring * fix docstring * wording fix Co-authored-by: Yael Ben-Haim <yaelbh@il.ibm.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Feedback from eggerdj - add default data processor - add data processor calibraiton - integrate run_fit and series fit - add data pre-processing (i.e. RB needs to take mean) - add fitter setup for initial guess and other options * readd 52c99ea and 0420d1d * add fit functions * lint * review by eggerdj r2 - change dp calibration logic - add gaussian function * add data processor keys in metadata add data processor keys in metadata * conform to #41 * feedback from chris1 - more general docstring - add default parameter to fit funcs * feedback from chris2 - parameter dict handling * feedback from chris3 - import path * lint * add fit option validation * add unittest and integration test * black & lint * simplify the analysis class * misc * add default figure generation * lint * add default value * fix docstring typo * remove outcome from default processing options * black and lint * add processor training check * fix pre processing routine * update rb analysis as an example - add num_qubits to curve analysis - add label generation to curve analysis * post process error handling when fit failed * add axis label * update axis formatting * more integration with new options * update irb * move class attributes to options * review comment from Chris * fix bug causes list indices must be ... * Analysis result formatting * update error docstring * lint * bug fix - reorder args of protected methods (series comes before xvals) - make analysis result list - relevant test fix - add filter kwargs to interleaved analysis * lint * fix spect analysis * fix composite analysis to use instance's analysis option * adjust curve figure appearance * black * update data and option handling * - private -> protected member - param list generation and validation in new method - all nan sigma handling - zero sigma handling in curve fitter - add data extraction utils - allow bounds = None - * update spectroscopy * black & lint * rerun rb notebook * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * - more docstring - add y normalization * black * add TODO comment * add analysis class information to result data * fix unittest * update spect analysis docstring * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * rb analysis class docstring * format docstring * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> Co-authored-by: Yael Ben-Haim <yaelbh@il.ibm.com> Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> Co-authored-by: Christopher J. Wood <cjwood@us.ibm.com>

* Update ExperimentData to handle figures Also makes some changes to be closer to draft API for terra base class * Update composite experiment class * Update RB and T1 experiments * Update tests * Fix linting errors * fix check for matplotlib * black * linting * Fix saving figure to file * Add type hints to BaseExperiment * Fix conflicts with main * unused import * fix doc string removed by rebase * Add handling of new and legacy backends

* wip curve fit analysis * wip curve fit analysis * curve_fit complete * - unittest - bug fix - lint * black * removed redundant code * fix unittest * fix docstring * fix docstring * wording fix Co-authored-by: Yael Ben-Haim <yaelbh@il.ibm.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Feedback from eggerdj - add default data processor - add data processor calibraiton - integrate run_fit and series fit - add data pre-processing (i.e. RB needs to take mean) - add fitter setup for initial guess and other options * readd 52c99ea and 0420d1d * add fit functions * lint * review by eggerdj r2 - change dp calibration logic - add gaussian function * add data processor keys in metadata add data processor keys in metadata * conform to qiskit-community#41 * feedback from chris1 - more general docstring - add default parameter to fit funcs * feedback from chris2 - parameter dict handling * feedback from chris3 - import path * lint * add fit option validation * add unittest and integration test * black & lint * simplify the analysis class * misc * add default figure generation * lint * add default value * fix docstring typo * remove outcome from default processing options * black and lint * add processor training check * fix pre processing routine * update rb analysis as an example - add num_qubits to curve analysis - add label generation to curve analysis * post process error handling when fit failed * add axis label * update axis formatting * more integration with new options * update irb * move class attributes to options * review comment from Chris * fix bug causes list indices must be ... * Analysis result formatting * update error docstring * lint * bug fix - reorder args of protected methods (series comes before xvals) - make analysis result list - relevant test fix - add filter kwargs to interleaved analysis * lint * fix spect analysis * fix composite analysis to use instance's analysis option * adjust curve figure appearance * black * update data and option handling * - private -> protected member - param list generation and validation in new method - all nan sigma handling - zero sigma handling in curve fitter - add data extraction utils - allow bounds = None - * update spectroscopy * black & lint * rerun rb notebook * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * - more docstring - add y normalization * black * add TODO comment * add analysis class information to result data * fix unittest * update spect analysis docstring * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> * rb analysis class docstring * format docstring * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> Co-authored-by: Yael Ben-Haim <yaelbh@il.ibm.com> Co-authored-by: Daniel Egger <38065505+eggerdj@users.noreply.github.com> Co-authored-by: Christopher J. Wood <cjwood@us.ibm.com>

yaelbh approved these changes May 9, 2021

View reviewed changes

yaelbh reviewed May 9, 2021

View reviewed changes

nkanazawa1989 reviewed May 12, 2021

View reviewed changes

chriseclectic added 12 commits May 13, 2021 11:17

Update ExperimentData to handle figures

1ff3e59

Also makes some changes to be closer to draft API for terra base class

Update composite experiment class

7e37528

Update RB and T1 experiments

c2347a0

Update tests

4bdb187

Fix linting errors

2d517fa

fix check for matplotlib

468d8fd

black

d38db9a

linting

db73475

Fix saving figure to file

e638ba7

Add type hints to BaseExperiment

e366cf8

Fix conflicts with main

9f43944

unused import

6613c0b

chriseclectic force-pushed the expdata branch from e020e50 to 6613c0b Compare May 13, 2021 15:18

chriseclectic added 2 commits May 13, 2021 11:23

fix doc string removed by rebase

24f49cb

Add handling of new and legacy backends

c351636

coruscating approved these changes May 13, 2021

View reviewed changes

coruscating merged commit 94c1eba into qiskit-community:main May 13, 2021

nkanazawa1989 pushed a commit to nkanazawa1989/qiskit-experiments that referenced this pull request May 13, 2021

conform to qiskit-community#41

fc67de7

nkanazawa1989 mentioned this pull request May 13, 2021

Curve Analysis class #46

Merged

coruscating added this to the Release 0.1 milestone Jun 16, 2021

chriseclectic deleted the expdata branch July 21, 2021 18:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update ExperimentData to store figures #41

Update ExperimentData to store figures #41

Uh oh!

chriseclectic commented May 6, 2021 •

edited

Loading

Uh oh!

yaelbh May 9, 2021

Uh oh!

chriseclectic May 10, 2021

Uh oh!

chriseclectic May 10, 2021

Uh oh!

yaelbh May 10, 2021

Uh oh!

nkanazawa1989 left a comment

Uh oh!

Uh oh!

nkanazawa1989 May 12, 2021

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Update ExperimentData to store figures #41

Update ExperimentData to store figures #41

Uh oh!

Conversation

chriseclectic commented May 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details and comments

Uh oh!

yaelbh May 9, 2021

Choose a reason for hiding this comment

Uh oh!

chriseclectic May 10, 2021

Choose a reason for hiding this comment

Uh oh!

chriseclectic May 10, 2021

Choose a reason for hiding this comment

Uh oh!

yaelbh May 10, 2021

Choose a reason for hiding this comment

Uh oh!

nkanazawa1989 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nkanazawa1989 May 12, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chriseclectic commented May 6, 2021 •

edited

Loading