Skip to content
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

Add artifacts interface #1342

Merged
merged 110 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from 101 commits
Commits
Show all changes
110 commits
Select commit Hold shift + click to select a range
fa675b4
Add new data model as a replacement of CurveData dataclass. This obje…
nkanazawa1989 Aug 14, 2023
fc68e24
Refactor the internals of curve fit. Figure creation code is now comb…
nkanazawa1989 Aug 14, 2023
4633b2f
Add pandas to requirements
nkanazawa1989 Aug 14, 2023
731b763
Update unittest for scatter table.
nkanazawa1989 Aug 18, 2023
07e85fc
Update type hint
nkanazawa1989 Aug 18, 2023
3f55ad9
Add release note
nkanazawa1989 Aug 18, 2023
458b98c
Introduce artifact data and replace analysis results for curve data a…
nkanazawa1989 Sep 11, 2023
0a37892
Introduce base class of experiment data DataCollection which doesn't …
nkanazawa1989 Sep 11, 2023
c942460
Move FigureData to dedicated file
nkanazawa1989 Sep 11, 2023
c22b3db
WIP: decouple service handling from ExperimentData.
nkanazawa1989 Sep 11, 2023
c7964f7
Update docstrings and merge some main changes
coruscating Nov 15, 2023
e4da036
result metadata parsing
coruscating Dec 12, 2023
f60ad6c
add test and some fixes
coruscating Dec 12, 2023
9b9c193
merge main
coruscating Jan 8, 2024
ed5cde2
revert experimentdata refactoring
coruscating Jan 9, 2024
ef376d3
fix bugs
coruscating Jan 9, 2024
65ad21b
merge main
coruscating Jan 9, 2024
f1a5671
fix curve analysis behavior and update tests
coruscating Jan 9, 2024
4e46084
fix tests and revert service handler
coruscating Jan 9, 2024
de3bc90
add release note and minor fixes
coruscating Jan 11, 2024
2dacc98
update serialization and add_artifact behavior, add test
coruscating Jan 16, 2024
1293fa0
fix flatten_results behavior
coruscating Jan 17, 2024
7f016c6
fix test
coruscating Jan 17, 2024
a755171
merge main
coruscating Jan 17, 2024
e71e962
move ArtifactData to containers
coruscating Jan 25, 2024
78a967a
Update internals of AnalysisResultTable
nkanazawa1989 Jan 10, 2024
7a83179
Update internals of ScatterTable
nkanazawa1989 Jan 18, 2024
3bc9525
Removed unused mixin
nkanazawa1989 Jan 18, 2024
68013a8
Fix index mismatch issue after JSON serialization
nkanazawa1989 Jan 31, 2024
5032e86
Add more tests
nkanazawa1989 Jan 31, 2024
7736f19
Bug fixes
nkanazawa1989 Jan 31, 2024
8bbaa15
Merge branch 'main' of github.com:Qiskit/qiskit-experiments into clea…
nkanazawa1989 Jan 31, 2024
fc9273e
Unpin pandas 2.2
nkanazawa1989 Jan 31, 2024
0cae116
Update old pattern
nkanazawa1989 Jan 31, 2024
0fc9d47
add equiv check for artifacts
coruscating Jan 31, 2024
db339d5
change upload format to zip
coruscating Feb 1, 2024
d0358e6
Sync with #1360
coruscating Feb 1, 2024
c7f8a4b
lint
coruscating Feb 1, 2024
8eeba4f
convert to dataclass
coruscating Feb 1, 2024
20d1a8e
add error handling
coruscating Feb 1, 2024
2fb28dc
Fix cross-reference
nkanazawa1989 Feb 1, 2024
83addf1
fix expdata.copy, add exp info to artifact
coruscating Feb 1, 2024
2a3a929
update tests
coruscating Feb 1, 2024
4fbf044
fix composite tests
coruscating Feb 1, 2024
c85e951
add artifact equality check
coruscating Feb 1, 2024
7c2a792
Address review comments and lint
coruscating Feb 1, 2024
4ea9f9e
delete unneeded code and update docstrings
coruscating Feb 1, 2024
838d944
move artifact names to exp metadata
coruscating Feb 1, 2024
3cf362e
revert autosave and update docs
coruscating Feb 1, 2024
296019d
lint
coruscating Feb 1, 2024
08ce7e6
lint
coruscating Feb 1, 2024
a08acdb
fix docs build
coruscating Feb 1, 2024
7ec4d24
fix curve analysis and add artifacts to intro tutorial
coruscating Feb 1, 2024
a3ef526
Merge remote-tracking branch 'upstream/main' into dataframe-pr3
coruscating Feb 1, 2024
ac972fd
Update curve analysis tutorial
nkanazawa1989 Feb 2, 2024
01471bb
Add shortcut methods
nkanazawa1989 Feb 2, 2024
8dc6c4f
Bugfix autosave
nkanazawa1989 Feb 2, 2024
144127a
Raise user warning when numbers contain multiple series
nkanazawa1989 Feb 2, 2024
a81f97c
Merge branch 'main' into cleanup/more_composition
nkanazawa1989 Feb 2, 2024
7c0662c
Bugfix: Missing circuit metadata in composite analysis
nkanazawa1989 Feb 2, 2024
543ce53
address review comments
coruscating Feb 2, 2024
9a7eeb7
bump service version
coruscating Feb 2, 2024
b11386b
Merge remote-tracking branch 'naoki/cleanup/more_composition' into da…
coruscating Feb 2, 2024
9973fd8
fix save order
coruscating Feb 2, 2024
b99f7cb
lint
coruscating Feb 2, 2024
92cfc92
Replace class_id with data_uid
nkanazawa1989 Feb 5, 2024
346d23a
Add documentation for filtering triplet
nkanazawa1989 Feb 5, 2024
2052a24
fix composite curve analysis
coruscating Feb 5, 2024
fc058a9
add howto and fix deletion behavior
coruscating Feb 5, 2024
120326f
update composite curve analysis
coruscating Feb 5, 2024
353e452
Merge remote-tracking branch 'naoki/cleanup/more_composition' into da…
coruscating Feb 5, 2024
ee03161
Apply review comments
nkanazawa1989 Feb 5, 2024
27ac6c7
update deletion behavior and tests
coruscating Feb 5, 2024
f962648
add artifactdata tests
coruscating Feb 5, 2024
079630a
update requirements
coruscating Feb 5, 2024
2cba94a
Merge branch 'main' into dataframe-pr3
coruscating Feb 5, 2024
337eeeb
lint and update docs
coruscating Feb 5, 2024
a5e4f44
Merge remote-tracking branch 'naoki/cleanup/more_composition' into da…
coruscating Feb 5, 2024
b1d3adf
Merge branch 'dataframe-pr3' of github.com:coruscating/qiskit-experim…
coruscating Feb 5, 2024
a6d7782
add integration tests
coruscating Feb 5, 2024
5be4e88
add more tests and add deprecation to docs
coruscating Feb 5, 2024
ca03a1d
add dataframes to docs with styling
coruscating Feb 6, 2024
ee5b34d
Wording suggestions
nkanazawa1989 Feb 6, 2024
38abdff
Remove DEFAULT_
nkanazawa1989 Feb 6, 2024
9e27f16
Reorganize the doc
nkanazawa1989 Feb 6, 2024
b870be3
Remove _data
nkanazawa1989 Feb 6, 2024
cc905c6
Remove key from add_data
nkanazawa1989 Feb 6, 2024
0dc4eb2
Remove type cast depending on the entry number
nkanazawa1989 Feb 6, 2024
f8c1efe
Minor docs formatting
nkanazawa1989 Feb 6, 2024
ee92f1d
Add more tests for result table
nkanazawa1989 Feb 6, 2024
03aac67
Performance optimization
nkanazawa1989 Feb 6, 2024
ac5bdd8
name, data_uid -> series_name, series_id
nkanazawa1989 Feb 6, 2024
3d41587
add test
coruscating Feb 6, 2024
b4efab4
Merge remote-tracking branch 'naoki/cleanup/more_composition' into da…
coruscating Feb 6, 2024
b9ae107
update tests to match changed filters
coruscating Feb 6, 2024
79370a7
add docs cross-link
coruscating Feb 6, 2024
4a41a70
Merge branch 'main' into dataframe-pr3
coruscating Feb 6, 2024
036e532
review comments
coruscating Feb 6, 2024
f74564b
add intro
coruscating Feb 6, 2024
eb89f37
update howto
coruscating Feb 7, 2024
f9a987e
Merge remote-tracking branch 'upstream/main' into dataframe-pr3
coruscating Feb 7, 2024
6b56b55
fix artifact copy and add test
coruscating Feb 7, 2024
83d33d4
fix release note
coruscating Feb 7, 2024
93b1298
review comments
coruscating Feb 7, 2024
a448bb8
Apply suggestions from code review
coruscating Feb 7, 2024
fe7485f
review comments
coruscating Feb 7, 2024
62184d0
minor fixes
coruscating Feb 7, 2024
2b3733e
update plot
coruscating Feb 7, 2024
74a65ba
put curve fit back in analysis results and deprecate access
coruscating Feb 7, 2024
f91bfe0
update deprecation message
coruscating Feb 8, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions docs/_static/dataframe.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/* Styling for pandas dataframes in documentation */

div.output table {
border: none;
border-collapse: collapse;
border-spacing: 0;
color: black;
font-size: 12px;
table-layout: fixed;
width: 100%;
}
div.output thead {
border-bottom: 1px solid black;
vertical-align: bottom;
}
div.output tr,
div.output th,
div.output td {
text-align: right;
vertical-align: middle;
padding: 0.5em 0.5em;
line-height: normal;
white-space: normal;
max-width: none;
border: none;
}
div.output th {
font-weight: bold;
}
div.output tbody tr:nth-child(odd) {
background: #f5f5f5;
}
div.output tbody tr:hover {
background: rgba(66, 165, 245, 0.2);
}
10 changes: 7 additions & 3 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,7 @@
templates_path = ["_templates"]
# Manually add the gallery CSS file for now
# TODO: Figure out why the styling is not working by default
html_css_files = [
"nbsphinx-gallery.css",
]
html_css_files = ["nbsphinx-gallery.css", "dataframe.css"]

nbsphinx_timeout = 360
nbsphinx_execute = os.getenv("QISKIT_DOCS_BUILD_TUTORIALS", "never")
Expand Down Expand Up @@ -171,6 +169,7 @@
"matplotlib": ("https://matplotlib.org/stable/", None),
"qiskit": ("https://docs.quantum.ibm.com/api/qiskit/", None),
"uncertainties": ("https://pythonhosted.org/uncertainties", None),
"pandas": ("http://pandas.pydata.org/docs/", None),
"qiskit_aer": ("https://qiskit.org/ecosystem/aer", None),
"qiskit_dynamics": ("https://qiskit.org/ecosystem/dynamics/", None),
"qiskit_ibm_runtime": ("https://docs.quantum.ibm.com/api/qiskit-ibm-runtime/", None),
Expand Down Expand Up @@ -236,6 +235,11 @@ def maybe_skip_member(app, what, name, obj, skip, options):
"filter_kwargs",
"fit_func",
"signature",
"artifact_id",
"artifact_data",
"device_components",
"created_time",
"data",
Comment on lines +238 to +242
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the hacky way to fix sphinx build errors that was introduced in #983.

]
skip_members = [
ParameterRepr.repr,
Expand Down
138 changes: 138 additions & 0 deletions docs/howtos/artifacts.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
Work with experiment artifacts
==============================

Problem
-------

You want to view, add, remove, and save artifacts associated with your :class:`.ExperimentData` instance.

Solution
--------

Artifacts are used to store auxiliary data for an experiment that don't fit neatly in the
:class:`.AnalysisResult` model. Any data that can be serialized, such as fit data, can be added as
:class:`.ArtifactData` artifacts to :class:`.ExperimentData`.

For example, after an experiment that uses :class:`.CurveAnalysis` is run, its :class:`.ExperimentData`
object is automatically populated with ``fit_summary`` and ``curve_data`` artifacts. The ``fit_summary``
artifact has one or more :class:`.CurveFitResult` objects that contain parameters from the fit. The
``curve_data`` artifact has a :class:`.ScatterTable` object that contain raw and fitted data in a pandas
coruscating marked this conversation as resolved.
Show resolved Hide resolved
:class:`~pandas:pandas.DataFrame`.

Viewing artifacts
~~~~~~~~~~~~~~~~~

Here we run a parallel experiment consisting of two :class:`.T1` experiments in parallel and then view the output
wshanks marked this conversation as resolved.
Show resolved Hide resolved
artifacts as a list of :class:`.ArtifactData` objects accessed by :meth:`.ExperimentData.artifacts`:

.. jupyter-execute::

from qiskit_ibm_runtime.fake_provider import FakePerth
from qiskit_aer import AerSimulator
from qiskit_experiments.library import T1
from qiskit_experiments.framework import ParallelExperiment
import numpy as np

backend = AerSimulator.from_backend(FakePerth())
exp1 = T1(physical_qubits=[0], delays=np.arange(1e-6, 6e-4, 5e-5))
exp2 = T1(physical_qubits=[1], delays=np.arange(1e-6, 6e-4, 5e-5))
data = ParallelExperiment([exp1, exp2], flatten_results=True).run(backend).block_for_results()
data.artifacts()

Artifacts can be accessed using either the artifact ID, which has to be unique in each
:class:`.ExperimentData` object, or the artifact name, which does not have to be unique and will return
all artifacts with the same name:

.. jupyter-execute::

print("Number of curve_data artifacts:", len(data.artifacts("curve_data")))
curve_data_id = data.artifacts("curve_data")[0].artifact_id
scatter_table = data.artifacts(curve_data_id).data
print("The first curve_data artifact:\n")
scatter_table.dataframe

The artifacts in a large composite experiment with ``flatten_results=True`` can be distinguished from
each other using the :attr:`~.ArtifactData.experiment` and :attr:`~.ArtifactData.device_components`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good follow up would be to add more query parameters to the artifacts method, so you can do exp_data.artifacts("curve_data", device_components=[Qubit(1)]) instead of filtering the list manually after getting exp_data.artifacts("curve_data"). Maybe there could be a shortcut like qubits=[1] so you don't need to import the Qubit class to do this query.

attributes.

One useful pattern is to load raw or fitted data from ``curve_data`` for further data manipulation. You
can work with the dataframe using standard pandas dataframe methods or the built-in
:class:`.ScatterTable` methods:

.. jupyter-execute::

import matplotlib.pyplot as plt

exp_type = data.artifacts(curve_data_id).experiment
component = data.artifacts(curve_data_id).device_components[0]

raw_data = scatter_table.filter(category="raw")
fitted_data = scatter_table.filter(category="fitted")

# visualize the data
fig, (ax1, ax2) = plt.subplots(1, 2)
ax1.errorbar(raw_data.x, raw_data.y, yerr=raw_data.y_err, capsize=5)
ax1.set_title(f"Raw data, {exp_type} experiment on {component}")
ax2.errorbar(fitted_data.x, fitted_data.y, yerr=fitted_data.y_err, capsize=5)
wshanks marked this conversation as resolved.
Show resolved Hide resolved
ax2.set_title(f"Fitted data, {exp_type} experiment on {component}")
plt.tight_layout()
plt.show()

Adding artifacts
~~~~~~~~~~~~~~~~

You can add arbitrary serializable data as an artifact.
coruscating marked this conversation as resolved.
Show resolved Hide resolved

.. jupyter-execute::

from qiskit_experiments.framework import ArtifactData

new_artifact = ArtifactData(name="experiment_notes", data={"content": "Testing some new ideas."})
data.add_artifacts(new_artifact)
data.artifacts("experiment_notes")

.. jupyter-execute::

print(data.artifacts("experiment_notes").data)

Saving and loading artifacts
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. note::
This feature is only for those who have access to the cloud service. You can
check whether you do by logging into the IBM Quantum interface
and seeing if you can see the `database <https://quantum.ibm.com/experiments>`__.

Artifacts are saved and loaded to and from the cloud service along with the rest of the
:class:`ExperimentData` object. Artifacts are stored as ``.zip`` files in the cloud service grouped by
the artifact name. For example, the composite experiment above will generate two artifact files, ``fit_summary.zip`` and
``curve_data.zip``. Each of these zipfiles will contain serialized artifact data in JSON format named
by their unique artifact ID:

.. jupyter-execute::
:hide-code:

print("fit_summary.zip")
print(f"|- {data.artifacts('fit_summary')[0].artifact_id}.json")
print(f"|- {data.artifacts('fit_summary')[1].artifact_id}.json")
print("curve_data.zip")
print(f"|- {data.artifacts('curve_data')[0].artifact_id}.json")
print(f"|- {data.artifacts('curve_data')[1].artifact_id}.json")
print("experiment_notes.zip")
print(f"|- {data.artifacts('experiment_notes').artifact_id}.json")

Note that for performance reasons, the auto save feature does not apply to artifacts. You must still
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want more save options here. Maybe some way to auto_save artifacts based on name or experiment name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think it would be nice to change auto_save to do a single upload of figures/results/artifacts in bulk once all analysis callbacks are complete as opposed to individual calls. Artifacts is the worst to upload one by one because of the zipped files but the others have room for performance improvement too.

call :meth:`.ExperimentData.save` once the experiment analysis has completed to upload artifacts to the
cloud service.

Note also though individual artifacts can be deleted, currently artifact files cannot be removed from the
cloud service. Instead, you can delete all artifacts of that name
using :meth:`~.delete_artifact` and then call :meth:`.ExperimentData.save`.
This will save an empty file to the service, and the loaded experiment data will not contain
these artifacts.

See Also
--------

* :ref:`Curve Analysis: Data management with scatter table <data_management_with_scatter_table>` tutorial
* :class:`.ScatterTable` API documentation
2 changes: 2 additions & 0 deletions docs/tutorials/curve_analysis.rst
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,8 @@ without an overhead of complex data management, as well as end-users with
retrieving and reusing the intermediate data for their custom fitting workflow
outside our curve fitting framework.
Note that a :class:`ScatterTable` instance may be saved in the :class:`.ExperimentData` as an artifact.
coruscating marked this conversation as resolved.
Show resolved Hide resolved
See the :doc:`Artifacts how-to </howtos/artifacts>` for more information.


.. _curve_analysis_workflow:

Expand Down
40 changes: 35 additions & 5 deletions docs/tutorials/getting_started.rst
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ analysis, respectively:
print(exp_data.job_status())
print(exp_data.analysis_status())

Figures
-------

Once the analysis is complete, figures are retrieved using the
:meth:`~.ExperimentData.figure` method. See the :doc:`visualization module
<visualization>` tutorial on how to customize figures for an experiment. For our
Expand All @@ -160,15 +163,22 @@ exponential decay model of the :math:`T_1` experiment:

display(exp_data.figure(0))

The fit results and associated parameters are accessed with
:meth:`~.ExperimentData.analysis_results`:
Analysis Results
----------------

The analysis results resulting from the fit are accessed with :meth:`~.ExperimentData.analysis_results`:

.. jupyter-execute::

for result in exp_data.analysis_results():
print(result)

Results can be indexed numerically (starting from 0) or using their name.
Results can be indexed numerically (starting from 0) or using their name. Analysis results can also be
retrieved in the pandas :class:`~pandas:pandas.DataFrame` format by passing ``dataframe=True``:

.. jupyter-execute::

exp_data.analysis_results(dataframe=True)

.. note::
See the :meth:`~.ExperimentData.analysis_results` API documentation for more
Expand All @@ -186,6 +196,24 @@ value and standard deviation of each value can be accessed as follows:
For further documentation on how to work with UFloats, consult the ``uncertainties``
:external+uncertainties:doc:`user_guide`.

Artifacts
---------

The curve fit data itself is contained in :meth:`~.ExperimentData.artifacts`, which is accessed
coruscating marked this conversation as resolved.
Show resolved Hide resolved
in an analogous manner. Artifacts for a standard experiment include both the curve fit data
stored in ``artifacts("curve_data")`` and information on the fit stored in ``artifacts("fit_summary")``.
Use the ``data`` attribute to access artifact data:

.. jupyter-execute::

print(exp_data.artifacts("fit_summary").data)

.. note::
See the :doc:`artifacts </howtos/artifacts>` how-to for more information on using artifacts.

Circuit data and metadata
-------------------------

Raw circuit output data and its associated metadata can be accessed with the
:meth:`~.ExperimentData.data` property. Data is indexed by the circuit it corresponds
to. Depending on the measurement level set in the experiment, the raw data will either
Expand All @@ -210,6 +238,9 @@ Experiments also have global associated metadata accessed by the

print(exp_data.metadata)

Job information
---------------

The actual backend jobs that were executed for the experiment can be accessed with the
:meth:`~.ExperimentData.jobs` method.

Expand Down Expand Up @@ -406,8 +437,7 @@ into one level:
)
parallel_data = parallel_exp.run(backend, seed_simulator=101).block_for_results()

for result in parallel_data.analysis_results():
print(result)
parallel_data.analysis_results(dataframe=True)

Broadcasting analysis options to child experiments
--------------------------------------------------
Expand Down
46 changes: 12 additions & 34 deletions qiskit_experiments/curve_analysis/base_curve_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,6 @@ class BaseCurveAnalysis(BaseAnalysis, ABC):
This method creates analysis results for important fit parameters
that might be defined by analysis options ``result_parameters``.

.. rubric:: _create_curve_data

This method creates analysis results for the formatted dataset, i.e. data used for the fitting.
Entries are created when the analysis option ``return_data_points`` is ``True``.
If analysis consists of multiple series, analysis result is created for
each curve data in the series definitions.

.. rubric:: _create_figures

This method creates figures by consuming the scatter table data.
Expand Down Expand Up @@ -162,9 +155,9 @@ def _default_options(cls) -> Options:
dataset without formatting, on canvas. This is ``False`` by default.
plot (bool): Set ``True`` to create figure for fit result or ``False`` to
not create a figure. This overrides the behavior of ``generate_figures``.
return_fit_parameters (bool): Set ``True`` to return all fit model parameters
with details of the fit outcome. Default to ``True``.
return_data_points (bool): Set ``True`` to include in the analysis result
return_fit_parameters (bool): (Deprecated) Set ``True`` to return all fit model parameters
with details of the fit outcome. Default to ``False``.
return_data_points (bool): (Deprecated) Set ``True`` to include in the analysis result
the formatted data points given to the fitter. Default to ``False``.
data_processor (Callable): A callback function to format experiment data.
This can be a :class:`.DataProcessor`
Expand Down Expand Up @@ -214,7 +207,7 @@ def _default_options(cls) -> Options:

options.plotter = CurvePlotter(MplDrawer())
options.plot_raw_data = False
options.return_fit_parameters = True
options.return_fit_parameters = False
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As evidenced by our tests, this could be a disruptive change. Personally, I never liked accessing the analysis results by number because it did not seem stable.

We should mention in an upgrade note that the result indices for CurveAnalysis experiments all shifted down 1. I wonder if it is worth producing a placeholder result for one version? ExperimentData.analysis_results() could check for that placeholder in index 0 and warn if it were called with a numerical index.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, ideally I would like to deprecate accessing results by number entirely. I also mentioned up above that the order of artifacts will change when you save and load a composite experiment: #1342 (comment).

In this case, I can shift simple indices by 1, but analysis_results() accepts slices and we'd have to introduce a bunch of code checking if the slice includes 0 and how to shift it. I'm not sure what the best solution is—the simplest is just to give a warning when users try to use numerical indices.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you want to shift all indices because only CurveAnalysis indices changed. That's most things but some like tomography do not use CurveAnalysis and would not have shifted.

Maybe we should not accept int for ExperimentData.artifacts()?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, based on #1385 it seems like we are treating the moving of the fit parameters from analysis results to artifacts as a minor non-core change. Does that seem right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I think I misunderstood you. I thought you wanted to put all the logic in expdata.analysis_results() but it would be easier to just add a placeholder in the actual AnalysisResultTable. Would the placeholder be actual data or a blank row? I think if we're going to get rid of numerical indices across the package, we could be more aggressive in making the breaking change now to get users used to the idea of not using them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was thinking something mostly blank. It could have a name like @Parameters_Placeholder or something like that and ExperimentData.analysis_results(0) could error if the result being returned were @Parameters_Placeholder. I think that is the only way the user gets a result directly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I think it's better to just put back the original behavior and warn the user if they try to use analysis_results(0) or numerical indices. I think it would confuse new users why there's a placeholder there with no useful information. One issue with moving to non-numerical indices is that we don't make the analysis result name very easy to find in the docs or regularized. Some are styled like Local Readout Mitigator and some are d_theta.

options.return_data_points = False
options.data_processor = None
options.normalization = False
Expand Down Expand Up @@ -246,37 +239,22 @@ def set_options(self, **fields):
Raises:
KeyError: When removed option ``curve_fitter`` is set.
"""
# TODO remove this in Qiskit Experiments v0.5

if "curve_fitter_options" in fields:
if "return_fit_parameters" in fields:
warnings.warn(
"The option 'curve_fitter_options' is replaced with 'lmfit_options.' "
"This option will be removed in Qiskit Experiments 0.5.",
"@Parameters_* result entry has moved to the experiment data artifact "
"regardless of option value. Setting this value doesn't affect result data.",
DeprecationWarning,
stacklevel=2,
)
fields["lmfit_options"] = fields.pop("curve_fitter_options")
del fields["return_fit_parameters"]

# TODO remove this in Qiskit Experiments 0.6
if "curve_drawer" in fields:
if "return_data_points" in fields:
warnings.warn(
"The option 'curve_drawer' is replaced with 'plotter'. "
"This option will be removed in Qiskit Experiments 0.6.",
"@Data_* result entry has moved to the experiment data artifact "
"regardless of option value. Setting this value doesn't affect result data.",
DeprecationWarning,
stacklevel=2,
)
# Set the plotter drawer to `curve_drawer`. If `curve_drawer` is the right type, set it
# directly. If not, wrap it in a compatibility drawer.
if isinstance(fields["curve_drawer"], BaseDrawer):
plotter = self.options.plotter
plotter.drawer = fields.pop("curve_drawer")
fields["plotter"] = plotter
else:
drawer = fields["curve_drawer"]
compat_drawer = LegacyCurveCompatDrawer(drawer)
plotter = self.options.plotter
plotter.drawer = compat_drawer
fields["plotter"] = plotter
del fields["return_data_points"]
coruscating marked this conversation as resolved.
Show resolved Hide resolved

super().set_options(**fields)

Expand Down
Loading
Loading