-
Notifications
You must be signed in to change notification settings - Fork 127
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
base: main
Are you sure you want to change the base?
Changes from 18 commits
183c18b
378b306
343eed5
872ac46
c172cac
20670a2
c0e3ec6
41bf46c
ab2e162
ef18ad9
3cb91bb
d095a82
bc4a29c
0bd48ae
31d73dd
6b128f0
7975955
15999a0
c2b4b5b
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 |
---|---|---|
|
@@ -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 | ||
|
@@ -29,6 +30,7 @@ | |
BaseAnalysis, | ||
ExperimentData, | ||
Options, | ||
AnalysisConfig, | ||
) | ||
from qiskit_experiments.visualization import ( | ||
BaseDrawer, | ||
|
@@ -422,3 +424,36 @@ def _initialize( | |
DeprecationWarning, | ||
) | ||
self.set_options(data_subfit_map=data_subfit_map) | ||
|
||
def config(self) -> AnalysisConfig: | ||
"""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. | ||
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. 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) | ||
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.
|
||
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
) | ||
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. If 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'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 |
||
# 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() | ||
|
@@ -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") | ||
|
@@ -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 | ||
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. What's the flow that will make |
||
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. " | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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): | ||
|
@@ -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) | ||
|
||
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. There is an issue with config handling when it comes to serializing a backend object when one is provided in the
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 | ||
|
@@ -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.") | ||
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. 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, | ||
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. If
I think this should be fixed - either don't allow an empty provider or update the doc string to explain the behavior when |
||
) -> "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" | ||
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. 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) | ||
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. Just out of curiosity, does the |
||
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, | ||
|
@@ -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. | ||
|
@@ -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)), | ||
|
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.
Why do we have
config
/from_config
with plotter handling specifically here inBaseCurveAnalysis
and not inBaseAnalysis
? What about analyzes not derived fromBaseCurveAnalysis
, e.g.TomographyAnalysis
orStarkP1SpectAnalysis
?