-
Notifications
You must be signed in to change notification settings - Fork 136
Save-load test and bug fixes #467
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
Changes from 6 commits
942fb0e
9105814
8651ebf
aadde6c
855032a
ad91683
6c7ed49
d5ffce1
bdb6720
fa4ce5e
ca52eff
6ae25d9
a3ce4c2
a0c72fd
6f0ccc8
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 |
|---|---|---|
|
|
@@ -871,10 +871,15 @@ def load(cls, experiment_id: str, service: DatabaseServiceV1) -> "DbExperimentDa | |
| notes=service_data.pop("notes"), | ||
| **service_data, | ||
| ) | ||
|
|
||
| if expdata.service is None: | ||
| expdata.service = service | ||
|
|
||
| # Retrieve analysis results | ||
| # Maybe this isn't necessary but the repr of the class should | ||
| # be updated to show correct number of results including remote ones | ||
| expdata._retrieve_analysis_results() | ||
|
|
||
| # mark it as existing in the DB | ||
| expdata._created_in_db = True | ||
| return expdata | ||
|
|
@@ -1326,6 +1331,8 @@ def _set_service(self, service: DatabaseServiceV1) -> None: | |
| if self._service: | ||
| raise DbExperimentDataError("An experiment service is already being used.") | ||
| self._service = service | ||
| for result in self._analysis_results.values(): | ||
| result.service = service | ||
|
Collaborator
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 the service of the results is not getting set correctly when loading from DB rather than fix it here it might be better to set the service in the
Collaborator
Author
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 don't understand. Setting the service here, when the experiment service is set, will affect any function that will query about the service, not just |
||
| with contextlib.suppress(Exception): | ||
| self.auto_save = self._service.options.get("auto_save", False) | ||
|
|
||
|
|
@@ -1393,7 +1400,6 @@ def __str__(self): | |
| ret += f"\nBackend: {self.backend}" | ||
| if self.tags: | ||
| ret += f"\nTags: {self.tags}" | ||
| ret += f"\nData: {len(self._data)}" | ||
|
Collaborator
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 think it's useful to know how much data is in the container (though I removed this whole repr method from the base DB class in another PR anyway, since if we want a user to only use ExperimentData we only need a pretty print for that class)
Collaborator
Author
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 returned it, but I think the phrasing should be clearer |
||
| ret += f"\nAnalysis Results: {n_res}" | ||
| ret += f"\nFigures: {len(self._figures)}" | ||
| ret += "\n" + line | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,226 @@ | ||
| # This code is part of Qiskit. | ||
| # | ||
| # (C) Copyright IBM 2021. | ||
| # | ||
| # This code is licensed under the Apache License, Version 2.0. You may | ||
| # obtain a copy of this license in the LICENSE.txt file in the root directory | ||
| # of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. | ||
| # | ||
| # Any modifications or derivative works of this code must retain this | ||
| # copyright notice, and modified files need to carry a notice indicating | ||
| # that they have been altered from the originals. | ||
|
|
||
| """Fake service class for tests.""" | ||
|
|
||
| from typing import Optional, List, Dict, Type, Any, Union, Tuple | ||
| import copy | ||
| import json | ||
|
|
||
| from test.fake_backend import FakeBackend | ||
|
|
||
| from qiskit_experiments.database_service import DatabaseServiceV1 | ||
| from qiskit_experiments.database_service.device_component import DeviceComponent | ||
|
|
||
| # pylint:disable=missing-raises-doc | ||
|
|
||
|
|
||
| class FakeService(DatabaseServiceV1): | ||
| """ | ||
| Extremely simple database for testing | ||
| """ | ||
|
|
||
| def __init__(self): | ||
| self.database = {} | ||
|
|
||
| def create_experiment( | ||
| self, | ||
| experiment_type: str, | ||
| backend_name: str, | ||
| metadata: Optional[Dict] = None, | ||
| experiment_id: Optional[str] = None, | ||
| parent_id: Optional[str] = None, | ||
| job_ids: Optional[List[str]] = None, | ||
| tags: Optional[List[str]] = None, | ||
| notes: Optional[str] = None, | ||
| json_encoder: Type[json.JSONEncoder] = json.JSONEncoder, | ||
| **kwargs: Any, | ||
| ) -> str: | ||
| """Create a new experiment in the database. | ||
|
|
||
| Args: | ||
| experiment_type: Experiment type. | ||
| backend_name: Name of the backend the experiment ran on. | ||
| metadata: Experiment metadata. | ||
| experiment_id: Experiment ID. It must be in the ``uuid4`` format. | ||
| One will be generated if not supplied. | ||
| parent_id: The experiment ID of the parent experiment. | ||
| The parent experiment must exist, must be on the same backend as the child, | ||
| and an experiment cannot be its own parent. | ||
| job_ids: IDs of experiment jobs. | ||
| tags: Tags to be associated with the experiment. | ||
| notes: Freeform notes about the experiment. | ||
| json_encoder: Custom JSON encoder to use to encode the experiment. | ||
| kwargs: Additional keywords supported by the service provider. | ||
|
|
||
| Returns: | ||
| Experiment ID. | ||
| """ | ||
|
|
||
| self.database[experiment_id] = { | ||
| "experiment_type": experiment_type, | ||
| "experiment_id": experiment_id, | ||
| "parent_id": parent_id, | ||
| "backend_name": backend_name, | ||
| "metadata": metadata, | ||
| "job_ids": job_ids, | ||
| "tags": tags, | ||
| "notes": notes, | ||
| "share_level": kwargs.get("share_level", None), | ||
| "figure_names": kwargs.get("figure_names", None), | ||
| "analysis": {}, | ||
| } | ||
|
|
||
| return experiment_id | ||
|
|
||
| def update_experiment( | ||
| self, | ||
| experiment_id: str, | ||
| metadata: Optional[Dict] = None, | ||
| job_ids: Optional[List[str]] = None, | ||
| notes: Optional[str] = None, | ||
| tags: Optional[List[str]] = None, | ||
| **kwargs: Any, | ||
| ) -> None: | ||
| """Update an existing experiment. | ||
|
|
||
| Args: | ||
| experiment_id: Experiment ID. | ||
| metadata: Experiment metadata. | ||
| job_ids: IDs of experiment jobs. | ||
| notes: Freeform notes about the experiment. | ||
| tags: Tags to be associated with the experiment. | ||
| kwargs: Additional keywords supported by the service provider. | ||
| """ | ||
| raise Exception("not implemented") | ||
|
|
||
| def experiment( | ||
| self, experiment_id: str, json_decoder: Type[json.JSONDecoder] = json.JSONDecoder | ||
| ) -> Dict: | ||
| """Retrieve a previously stored experiment. | ||
|
|
||
| Args: | ||
| experiment_id: Experiment ID. | ||
| json_decoder: Custom JSON decoder to use to decode the retrieved experiment. | ||
|
|
||
| Returns: | ||
| A dictionary containing the retrieved experiment data. | ||
| """ | ||
|
|
||
| db_entry = copy.deepcopy(self.database[experiment_id]) | ||
| db_entry["backend"] = FakeBackend() | ||
| return db_entry | ||
|
|
||
| def experiments( | ||
| self, | ||
| limit: Optional[int] = 10, | ||
| json_decoder: Type[json.JSONDecoder] = json.JSONDecoder, | ||
| device_components: Optional[Union[str, DeviceComponent]] = None, | ||
| experiment_type: Optional[str] = None, | ||
| backend_name: Optional[str] = None, | ||
| tags: Optional[List[str]] = None, | ||
| parent_id: Optional[str] = None, | ||
| tags_operator: Optional[str] = "OR", | ||
| **filters: Any, | ||
| ) -> List[Dict]: | ||
| raise Exception("not implemented") | ||
|
|
||
| def delete_experiment(self, experiment_id: str) -> None: | ||
| raise Exception("not implemented") | ||
|
|
||
| def create_analysis_result( | ||
| self, | ||
| experiment_id: str, | ||
| result_data: Dict, | ||
| result_type: str, | ||
| device_components: Optional[Union[str, DeviceComponent]] = None, | ||
| tags: Optional[List[str]] = None, | ||
| quality: Optional[str] = None, | ||
| verified: bool = False, | ||
| result_id: Optional[str] = None, | ||
| json_encoder: Type[json.JSONEncoder] = json.JSONEncoder, | ||
| **kwargs: Any, | ||
| ) -> str: | ||
| self.database[experiment_id]["analysis"][result_id] = { | ||
| "result_data": result_data, | ||
| "result_id": result_id, | ||
| "result_type": result_type, | ||
| "device_components": device_components, | ||
| "experiment_id": experiment_id, | ||
| "quality": quality, | ||
| "verified": verified, | ||
| "tags": tags, | ||
| "service": self, | ||
| } | ||
|
|
||
| return result_id | ||
|
|
||
| def update_analysis_result( | ||
| self, | ||
| result_id: str, | ||
| result_data: Optional[Dict] = None, | ||
| tags: Optional[List[str]] = None, | ||
| quality: Optional[str] = None, | ||
| verified: bool = None, | ||
| **kwargs: Any, | ||
| ) -> None: | ||
| raise Exception("not implemented") | ||
|
|
||
| def analysis_result( | ||
| self, result_id: str, json_decoder: Type[json.JSONDecoder] = json.JSONDecoder | ||
| ) -> Dict: | ||
| raise Exception("not implemented") | ||
|
|
||
| def analysis_results( | ||
| self, | ||
| limit: Optional[int] = 10, | ||
| json_decoder: Type[json.JSONDecoder] = json.JSONDecoder, | ||
| device_components: Optional[Union[str, DeviceComponent]] = None, | ||
| experiment_id: Optional[str] = None, | ||
| result_type: Optional[str] = None, | ||
| backend_name: Optional[str] = None, | ||
| quality: Optional[str] = None, | ||
| verified: Optional[bool] = None, | ||
| tags: Optional[List[str]] = None, | ||
| tags_operator: Optional[str] = "OR", | ||
| **filters: Any, | ||
| ) -> List[Dict]: | ||
| return self.database[experiment_id]["analysis"].values() | ||
|
|
||
| def delete_analysis_result(self, result_id: str) -> None: | ||
| raise Exception("not implemented") | ||
|
|
||
| def create_figure( | ||
| self, experiment_id: str, figure: Union[str, bytes], figure_name: Optional[str] | ||
| ) -> Tuple[str, int]: | ||
| return | ||
|
|
||
| def update_figure( | ||
| self, experiment_id: str, figure: Union[str, bytes], figure_name: str | ||
| ) -> Tuple[str, int]: | ||
| raise Exception("not implemented") | ||
|
|
||
| def figure( | ||
| self, experiment_id: str, figure_name: str, file_name: Optional[str] = None | ||
| ) -> Union[int, bytes]: | ||
| raise Exception("not implemented") | ||
|
|
||
| def delete_figure( | ||
| self, | ||
| experiment_id: str, | ||
| figure_name: str, | ||
| ) -> None: | ||
| raise Exception("not implemented") | ||
|
|
||
| @property | ||
| def preferences(self) -> Dict: | ||
| return {"auto_save": False} |
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.
The extra field is not what is saved in the chisq of the analysis result, this is just leftover metadata from certain curve fitting experiments. It should just be
result_data.pop("chisq", None)here, but since we apply thedisplay_formatfunction for modifying chisq for UI display we will need to add a line to save the original without modification.I think the correct thing to do here is update line 155 of the
savefunction to store the non-modified chisq:then this line should be
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.
I did as you suggest and it's working. But I think the correct thing would be (in another PR) to add
chisqas a parameter increate_analysis_resultandupdate_analysis_result. Is there a reason forchisqto be different from other attributes, likequalityandverified?