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

Save experiment and analysis config as artifacts #1424

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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 qiskit_experiments/curve_analysis/base_curve_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
Base class of curve analysis.
"""

from collections import OrderedDict
import warnings
from abc import ABC, abstractmethod
from typing import Dict, List, Union
Expand All @@ -29,6 +30,7 @@
BaseAnalysis,
ExperimentData,
Options,
AnalysisConfig,
)
from qiskit_experiments.visualization import (
BaseDrawer,
Expand Down Expand Up @@ -422,3 +424,36 @@ def _initialize(
DeprecationWarning,
)
self.set_options(data_subfit_map=data_subfit_map)

def config(self) -> AnalysisConfig:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have config/from_config with plotter handling specifically here in BaseCurveAnalysis and not in BaseAnalysis? What about analyzes not derived from BaseCurveAnalysis, e.g. TomographyAnalysis or StarkP1SpectAnalysis?

"""Return the config dataclass for this analysis. We replicate the method from `BaseAnalysis`
because we cannot directly modify the returned object. This limitation arises from our use of
it hashable we use `dataclasses.dataclass(frozen=True)` to ensure its hashability.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The last sentence is grammatically incorrect (and indeed it's not clear what it's trying to say).

Additionally, we include the `plotter` and `drawer` options, which are specific to
this context."""
args = tuple(getattr(self, "__init_args__", OrderedDict()).values())
kwargs = dict(getattr(self, "__init_kwargs__", OrderedDict()))
# Only store non-default valued options
options = dict((key, getattr(self._options, key)) for key in self._set_options)
Copy link
Collaborator

Choose a reason for hiding this comment

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

BaseAnalysis and BaseCurveAnalysis should call a common helper function, instead of duplicating the code. This is possible in spite of everything being hashable.

if isinstance(self.plotter, BasePlotter):
plotter = self.plotter.config()
else:
plotter = None

return AnalysisConfig(
cls=type(self),
args=args,
kwargs=kwargs,
options=options,
plotter=plotter,
)

@classmethod
def from_config(cls, config: Union[AnalysisConfig, Dict]) -> "BaseCurveAnalysis":
"""Initialize a curve analysis class from analysis config"""
if isinstance(config, dict):
config = AnalysisConfig(**config)
ret = super().from_config(config)
if config.plotter:
ret.options.plotter = config.plotter.plotter()
return ret
9 changes: 9 additions & 0 deletions qiskit_experiments/framework/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@
BackendTiming
RestlessMixin

Helper Functions
****************

.. autosummary::
:toctree: ../stubs/

load_all

"""
from qiskit.providers.options import Options
from qiskit_experiments.framework.backend_data import BackendData
Expand Down Expand Up @@ -155,3 +163,4 @@
)
from .json import ExperimentEncoder, ExperimentDecoder
from .restless_mixin import RestlessMixin
from qiskit_experiments.framework.helpers import load_all
27 changes: 18 additions & 9 deletions qiskit_experiments/framework/base_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ def run(
will be returned containing only the new analysis results and figures.
This data can then be saved as its own experiment to a database service.
"""
experiment_data.add_artifacts(
ArtifactData(name="analysis_config", data=self.config()),
overwrite_name=True,
overwrite_id=True,
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If replace_results is False then we create a copy of the ExperimentData object. Both the original object and its copy will now contain the analysis config artifact, although the original one will not contain the corresponding analysis result.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm also concerned about the old analysis config that's stored in the experiment data and is now overridden. The concern applies for the two possible values of replace_results.

# Make a new copy of experiment data if not updating results
if not replace_results and _requires_copy(experiment_data):
experiment_data = experiment_data.copy()
Expand All @@ -166,8 +171,7 @@ def run(
analysis.set_options(**options)

def run_analysis(expdata: ExperimentData):
# Clearing previous analysis data
experiment_data._clear_results()
experiment_data._clear_results(delete_artifacts=False)

if not expdata.data():
warnings.warn("ExperimentData object data is empty.\n")
Expand Down Expand Up @@ -201,13 +205,18 @@ def run_analysis(expdata: ExperimentData):

expdata.add_analysis_results(**table_format)
elif isinstance(result, ArtifactData):
if not result.experiment_id:
result.experiment_id = expdata.experiment_id
if not result.device_components:
result.device_components = analysis._get_experiment_components(expdata)
if not result.experiment:
result.experiment = expdata.experiment_type
expdata.add_artifacts(result)
if (
result.name != "analysis_config"
): # Not saving constituent analysis configs
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the flow that will make _run_analysis return an analysis config?

if not result.experiment_id:
result.experiment_id = expdata.experiment_id
if not result.device_components:
result.device_components = analysis._get_experiment_components(
expdata
)
if not result.experiment:
result.experiment = expdata.experiment_type
expdata.add_artifacts(result)
else:
raise TypeError(
f"Invalid object type {result.__class__.__name__} for analysis results. "
Expand Down
110 changes: 106 additions & 4 deletions qiskit_experiments/framework/base_experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@
"""
Base Experiment class.
"""

import warnings
from abc import ABC, abstractmethod
import copy
from collections import OrderedDict
import logging
import traceback
from typing import Sequence, Optional, Tuple, List, Dict, Union

from qiskit_ibm_experiment import IBMExperimentService
from qiskit import transpile, QuantumCircuit
from qiskit.providers import Job, Backend
from qiskit.providers import Provider, Job, Backend
from qiskit.exceptions import QiskitError
from qiskit.qobj.utils import MeasLevel
from qiskit.providers.options import Options
Expand All @@ -28,7 +31,12 @@
from qiskit_experiments.framework.base_analysis import BaseAnalysis
from qiskit_experiments.framework.experiment_data import ExperimentData
from qiskit_experiments.framework.configs import ExperimentConfig
from qiskit_experiments.framework.json import ExperimentDecoder
from qiskit_experiments.framework.package_deps import qiskit_version
from qiskit_experiments.database_service import Qubit
from qiskit_experiments.database_service.utils import zip_to_objs

LOG = logging.getLogger(__name__)


class BaseExperiment(ABC, StoreInitArgs):
Expand Down Expand Up @@ -171,13 +179,20 @@ def config(self) -> ExperimentConfig:
(key, getattr(self._transpile_options, key)) for key in self._set_transpile_options
)
run_options = dict((key, getattr(self._run_options, key)) for key in self._set_run_options)

Copy link
Contributor

Choose a reason for hiding this comment

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

There is an issue with config handling when it comes to serializing a backend object when one is provided in the __init__ method. In this case, the backend is included in the config e.g. via kwargs and is serialized in way that upon deserialization (through BaseExperiment.load) the following warning is emitted:

/tmp/qiskit-experiments/qiskit_experiments/framework/json.py:581: UserWarning: Could not deserialize instance of class <class 'qiskit_ibm_runtime.ibm_backend.IBMBackend'> from settings {}. Value is {'class': <class 'qiskit_ibm_runtime.ibm_backend.IBMBackend'>, 'settings': {}, 'version': '0.23.0'}
The following exception was raised:
Traceback (most recent call last):
File "/tmp/qiskit-experiments/qiskit_experiments/framework/json.py", line 338, in _deserialize_object
return cls(**settings)
TypeError: IBMBackend.init() missing 3 required positional arguments: 'configuration', 'service', and 'api_client'

return _deserialize_object(obj_val)

The experiment still loads, but this warning is annoying.

if isinstance(self.analysis, BaseAnalysis):
analysis_config = self.analysis.config()
else:
analysis_config = None

return ExperimentConfig(
cls=type(self),
args=args,
kwargs=kwargs,
experiment_options=experiment_options,
transpile_options=transpile_options,
run_options=run_options,
analysis=analysis_config,
)

@classmethod
Expand All @@ -192,8 +207,94 @@ def from_config(cls, config: Union[ExperimentConfig, Dict]) -> "BaseExperiment":
ret.set_transpile_options(**config.transpile_options)
if config.run_options:
ret.set_run_options(**config.run_options)
if config.analysis:
ret.analysis = config.analysis.analysis()
return ret

def save(self, service=None, backend=None):
"""Save an experiment without running it."""
if not service:
raise QiskitError("A service must be provided to save the experiment.")
if not self.backend and not backend:
raise QiskitError("Backend must be set to save the experiment.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the backend mandatory?

exp_data = self._initialize_experiment_data(
service=service, backend=backend or self.backend
)
exp_data.save()
return exp_data

@classmethod
def load(
cls,
experiment_id: str,
service: Optional[IBMExperimentService] = None,
provider: Optional[Provider] = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

If service it not None, but provider is None, we get this warning:

/tmp/qiskit-experiments/qiskit_experiments/framework/base_experiment.py:286: UserWarning: Unable to retrieve backend.
warnings.warn("Unable to retrieve backend.")

I think this should be fixed - either don't allow an empty provider or update the doc string to explain the behavior when provider is empty. Currently the doc string explicitly says that IBMProvider is required

) -> "BaseExperiment":
"""Load a saved experiment from a database service.

Args:
experiment_id: Experiment ID.
service: the database service.
provider: an IBMProvider required for loading the experiment data and
can be used to initialize the service. When using
:external+qiskit_ibm_runtime:doc:`qiskit-ibm-runtime <index>`,
this is the :class:`~qiskit_ibm_runtime.QiskitRuntimeService` and should
not be confused with the experiment database service
:meth:`qiskit_ibm_experiment.IBMExperimentService`.

Returns:
The reconstructed experiment.
Raises:
QiskitError: If the experiment could not be reconstructed.
"""
if service is None:
if provider is None:
raise QiskitError(
"Loading an experiment requires a valid Qiskit provider or experiment service."
)
service = ExperimentData.get_service_from_provider(provider)

data = service.experiment(experiment_id, json_decoder=ExperimentDecoder)

# Recreate artifacts
experiment_config_filename = "experiment_config.zip"
Copy link
Contributor

Choose a reason for hiding this comment

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

Better store the name in one place to be used here and in `ExperimentData.init" (maybe in helpers.py to avoid cyclic dependency)


if experiment_config_filename in data.metadata["artifact_files"]:
if service.experiment_has_file(experiment_id, experiment_config_filename):
artifact_file = service.file_download(experiment_id, experiment_config_filename)

experiment_config = next(
zip_to_objs(artifact_file, json_decoder=ExperimentDecoder)
).data

exp_versions = data.metadata["_source"]["qiskit_version"]
cur_versions = qiskit_version()
if exp_versions != cur_versions:
warnings.warn(
f"The experiment was created with {exp_versions}, "
f"but you have versions {cur_versions}."
)
try:
reconstructed_experiment = experiment_config.cls.from_config(experiment_config)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just out of curiosity, does the cls here have any effect? Or is it only for readability?

except Exception as exc: # pylint: disable=broad-except:
raise QiskitError("Recreating experiment failed with {Exception}.") from exc
backend_name = data.backend
if backend_name:
try:
reconstructed_experiment.backend = provider.get_backend(backend_name)
except Exception: # pylint: disable=broad-except
warnings.warn("Unable to retrieve backend.")
else:
warnings.warn("No backend specified in loaded experiment.")
return reconstructed_experiment
else:
raise QiskitError("The experiment doesn't have saved metadata in the service.")
else:
raise QiskitError(
f"No '{experiment_config_filename}' field in the experiment metadata. can't load "
f"the experiment."
)

def run(
self,
backend: Optional[Backend] = None,
Expand Down Expand Up @@ -260,9 +361,9 @@ def run(
else:
return experiment_data

def _initialize_experiment_data(self) -> ExperimentData:
def _initialize_experiment_data(self, **kwargs) -> ExperimentData:
"""Initialize the return data container for the experiment run"""
return ExperimentData(experiment=self)
return ExperimentData(experiment=self, **kwargs)

def _finalize(self):
"""Finalize experiment object before running jobs.
Expand Down Expand Up @@ -475,6 +576,7 @@ def _metadata(self) -> Dict[str, any]:
By default, this assumes the experiment is running on qubits only. Subclasses can override
this method to add custom experiment metadata to the returned experiment result data.
"""

metadata = {
"physical_qubits": list(self.physical_qubits),
"device_components": list(map(Qubit, self.physical_qubits)),
Expand Down
Loading
Loading