diff --git a/qiskit_experiments/framework/base_analysis.py b/qiskit_experiments/framework/base_analysis.py index 167e07e319..b0ca658418 100644 --- a/qiskit_experiments/framework/base_analysis.py +++ b/qiskit_experiments/framework/base_analysis.py @@ -184,7 +184,7 @@ def run_analysis(expdata: ExperimentData): if not result.experiment: result.experiment = expdata.experiment_type if not result.device_components: - result.device_components = self._get_experiment_components(expdata) + result.device_components = analysis._get_experiment_components(expdata) if not result.backend: result.backend = expdata.backend_name if not result.created_time: @@ -204,7 +204,7 @@ def run_analysis(expdata: ExperimentData): if not result.experiment_id: result.experiment_id = expdata.experiment_id if not result.device_components: - result.device_components = self._get_experiment_components(expdata) + result.device_components = analysis._get_experiment_components(expdata) if not result.experiment: result.experiment = expdata.experiment_type expdata.add_artifacts(result) @@ -227,7 +227,7 @@ def run_analysis(expdata: ExperimentData): name=f"{expdata.experiment_type}_{qubits_repr}_{short_id}.svg", ) figure_to_add.append(figure) - expdata.add_figures(figure_to_add, figure_names=self.options.figure_names) + expdata.add_figures(figure_to_add, figure_names=analysis.options.figure_names) experiment_data.add_analysis_callback(run_analysis) diff --git a/qiskit_experiments/framework/experiment_data.py b/qiskit_experiments/framework/experiment_data.py index 3bfe764718..62fe646aca 100644 --- a/qiskit_experiments/framework/experiment_data.py +++ b/qiskit_experiments/framework/experiment_data.py @@ -630,6 +630,7 @@ def _clear_results(self): self._deleted_figures.append(key) self._figures = ThreadSafeOrderedDict() self._artifacts = ThreadSafeOrderedDict() + self._db_data.figure_names.clear() @property def service(self) -> Optional[IBMExperimentService]: diff --git a/releasenotes/notes/figure-names-inconsistency-afca1ac8e00fabac.yaml b/releasenotes/notes/figure-names-inconsistency-afca1ac8e00fabac.yaml new file mode 100644 index 0000000000..f7066e4c1d --- /dev/null +++ b/releasenotes/notes/figure-names-inconsistency-afca1ac8e00fabac.yaml @@ -0,0 +1,14 @@ +--- +fixes: + - | + :class:`.ExperimentData` was updated so that running analysis a second time + with ``replace_results=True`` does not result in the ``figure_names`` + property having incorrect data (both old and new figure names if the names + changed). See `#1430 + `__. + - | + :class:`.BaseAnalysis` was updated to respect ``figure_names`` as a keyword + argument to the ``run()`` method. Previously, this argument was ignored and + ``figure_names`` could only be set as an analysis option prior to calling + ``run()``. See `#1430 + `__. diff --git a/test/database_service/test_db_experiment_data.py b/test/database_service/test_db_experiment_data.py index 020e871cc7..d9347a6377 100644 --- a/test/database_service/test_db_experiment_data.py +++ b/test/database_service/test_db_experiment_data.py @@ -1067,6 +1067,7 @@ def test_copy_metadata(self): def test_copy_figure_artifacts(self): """Test copy expdata figures and artifacts.""" exp_data = FakeExperiment(experiment_type="qiskit_test").run(backend=FakeBackend()) + exp_data.block_for_results() exp_data.add_figures(str.encode("hello world")) exp_data.add_artifacts(ArtifactData(name="test", data="foo")) copied = exp_data.copy(copy_results=True) diff --git a/test/framework/test_framework.py b/test/framework/test_framework.py index 6d45fe2f67..1a2ae395d3 100644 --- a/test/framework/test_framework.py +++ b/test/framework/test_framework.py @@ -28,6 +28,7 @@ from qiskit_experiments.exceptions import AnalysisError from qiskit_experiments.framework import ( ExperimentData, + FigureData, BaseExperiment, BaseAnalysis, AnalysisResultData, @@ -163,6 +164,30 @@ def test_analysis_replace_results_true(self): self.assertEqualExtended(expdata1.analysis_results(), expdata2.analysis_results()) self.assertEqual(result_ids, list(expdata2._deleted_analysis_results)) + def test_analysis_replace_results_true_new_figure(self): + """Test running analysis with replace_results=True keeps figure data consistent""" + analysis = FakeAnalysis() + analysis.options.add_figures = True + analysis.options.figure_names = ["old_figure_name.svg"] + + expdata = ExperimentData() + expdata.add_data(self.fake_job_data()) + analysis.run(expdata, seed=54321) + self.assertExperimentDone(expdata) + + # Assure all figure names map to valid figures + self.assertEqual(expdata.figure_names, ["old_figure_name.svg"]) + self.assertIsInstance(expdata.figure("old_figure_name"), FigureData) + + analysis.run( + expdata, replace_results=True, seed=12345, figure_names=["new_figure_name.svg"] + ) + self.assertExperimentDone(expdata) + + # Assure figure names have changed but are still valid + self.assertEqual(expdata.figure_names, ["new_figure_name.svg"]) + self.assertIsInstance(expdata.figure("new_figure_name"), FigureData) + def test_analysis_replace_results_false(self): """Test running analysis with replace_results=False""" analysis = FakeAnalysis()