From 2f23f4d16690f000fe5c6cb0959c27d3bc6ec7b6 Mon Sep 17 00:00:00 2001 From: Helena Zhang Date: Wed, 3 May 2023 16:14:15 -0400 Subject: [PATCH 1/9] updated figure names --- .../framework/experiment_data.py | 34 +++++++++++-------- .../update-figure-name-2db258c30ffe9912.yaml | 10 ++++++ .../test_db_experiment_data.py | 2 +- 3 files changed, 30 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml diff --git a/qiskit_experiments/framework/experiment_data.py b/qiskit_experiments/framework/experiment_data.py index 5a79d2d98f..2c4c58b183 100644 --- a/qiskit_experiments/framework/experiment_data.py +++ b/qiskit_experiments/framework/experiment_data.py @@ -998,27 +998,27 @@ def data( @do_auto_save def add_figures( self, - figures, - figure_names=None, - overwrite=False, - save_figure=None, + figures: Union[str, bytes, pyplot.Figure, list], + figure_names: Union[str, list] = None, + overwrite: bool = False, + save_figure: bool = None, ) -> Union[str, List[str]]: """Add the experiment figure. Args: - figures (str or bytes or pyplot.Figure or list): Paths of the figure - files or figure data. - figure_names (str or list): Names of the figures. If ``None``, use the figure file - names, if given, or a generated name. If `figures` is a list, then - `figure_names` must also be a list of the same length or ``None``. - overwrite (bool): Whether to overwrite the figure if one already exists with + figures: Paths of the figure files or figure data. + figure_names: Names of the figures. If ``None``, use the figure file + names, if given, or a generated name of the format ``experiment_type``, figure + index, first 5 elements of ``physical_qubits``, and first 8 digits of the experiment + ID connected by underscores. If `figures` is a list, then `figure_names` must also + be a list of the same length or ``None``. + overwrite: Whether to overwrite the figure if one already exists with the same name. - save_figure (bool): Whether to save the figure in the database. If ``None``, + save_figure: Whether to save the figure in the database. If ``None``, the ``auto-save`` attribute is used. Returns: - str or list: - Figure names. + Figure names. Raises: ExperimentEntryExists: If the figure with the same name already exists, @@ -1044,6 +1044,7 @@ def add_figures( fig_name = ( f"{self.experiment_type}_" f"Fig-{len(self._figures)}_" + f'{"_".join(f"Q{i}" for i in self.metadata.get("physical_qubits")[:5])}_' f"Exp-{self.experiment_id[:8]}.svg" ) else: @@ -1067,10 +1068,13 @@ def add_figures( # check whether the figure is already wrapped, meaning it came from a sub-experiment if isinstance(figure, FigureData): - figure_data = figure.copy(new_name=fig_name) + figure_data = figure.copy(new_name=figure.name) else: - figure_metadata = {"qubits": self.metadata.get("physical_qubits")} + figure_metadata = { + "qubits": self.metadata.get("physical_qubits"), + "experiment_type": self.experiment_type, + } figure_data = FigureData(figure=figure, name=fig_name, metadata=figure_metadata) self._figures[fig_name] = figure_data diff --git a/releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml b/releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml new file mode 100644 index 0000000000..60c86b3a1d --- /dev/null +++ b/releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml @@ -0,0 +1,10 @@ +--- +other: + - | + Figure names have been updated to include qubit indices up to the first five physical qubits in + the experiment, with format ``StandardRB_Fig-0_Q0_Q1_Q2_Q3_Q5_Exp-b4f1d8ad.svg``. For composite + experiments where ``flatten_results`` is set to ``True``, the head of the figure name is now the + class name of the experiment instead of ``ParallelExperiment`` or ``BatchExperiment``, such that + the figure name is the same when ``flatten_results`` is ``False``. + - | + Figure metadata now includes ``experiment_type``. diff --git a/test/database_service/test_db_experiment_data.py b/test/database_service/test_db_experiment_data.py index 0c804061e3..15e2dae241 100644 --- a/test/database_service/test_db_experiment_data.py +++ b/test/database_service/test_db_experiment_data.py @@ -350,7 +350,7 @@ def test_add_figure_metadata(self): self.assertEqual(figure_data.metadata["qubits"], qubits) self.assertEqual(figure_data.metadata["foo"], "bar") - expected_name_prefix = "qiskit_test_Fig-0_Exp-" + expected_name_prefix = "qiskit_test_Fig-0_Q0_Q1_Q2_Exp-" self.assertEqual(figure_data.name[: len(expected_name_prefix)], expected_name_prefix) exp_data2 = ExperimentData( From 319bc98cbb52f8844909e2e08d827309cd051b77 Mon Sep 17 00:00:00 2001 From: Helena Zhang Date: Wed, 3 May 2023 17:16:51 -0400 Subject: [PATCH 2/9] fix logic where figure names are specified --- qiskit_experiments/framework/experiment_data.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/qiskit_experiments/framework/experiment_data.py b/qiskit_experiments/framework/experiment_data.py index 2c4c58b183..d264be12d9 100644 --- a/qiskit_experiments/framework/experiment_data.py +++ b/qiskit_experiments/framework/experiment_data.py @@ -1040,13 +1040,15 @@ def add_figures( if figure_names is None: if isinstance(figure, str): fig_name = figure - else: + elif not isinstance(figure, FigureData): fig_name = ( f"{self.experiment_type}_" f"Fig-{len(self._figures)}_" - f'{"_".join(f"Q{i}" for i in self.metadata.get("physical_qubits")[:5])}_' + f'{"_".join(f"Q{i}" for i in self.metadata.get("physical_qubits", [])[:5])}_' f"Exp-{self.experiment_id[:8]}.svg" ) + else: + fig_name = figure.name else: fig_name = figure_names[idx] @@ -1068,7 +1070,7 @@ def add_figures( # check whether the figure is already wrapped, meaning it came from a sub-experiment if isinstance(figure, FigureData): - figure_data = figure.copy(new_name=figure.name) + figure_data = figure.copy(new_name=fig_name) else: figure_metadata = { From 7c8c353da7d043c8ad565aa2a17faa207b9b1af2 Mon Sep 17 00:00:00 2001 From: Helena Zhang Date: Thu, 25 May 2023 12:36:41 -0400 Subject: [PATCH 3/9] update docstring with new behavior --- qiskit_experiments/framework/experiment_data.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/qiskit_experiments/framework/experiment_data.py b/qiskit_experiments/framework/experiment_data.py index d264be12d9..c10c83a0a3 100644 --- a/qiskit_experiments/framework/experiment_data.py +++ b/qiskit_experiments/framework/experiment_data.py @@ -1013,7 +1013,10 @@ def add_figures( ID connected by underscores. If `figures` is a list, then `figure_names` must also be a list of the same length or ``None``. overwrite: Whether to overwrite the figure if one already exists with - the same name. + the same name. By default, overwrite is ``False`` and the figure will be renamed + with an incrementing numerical suffix. For example, trying to save ``figure.svg`` when + ``figure.svg`` already exists will save it as ``figure-1.svg``, and trying to save + ``figure-1.svg`` when ``figure-1.svg`` already exists will save it as ``figure-2.svg``. save_figure: Whether to save the figure in the database. If ``None``, the ``auto-save`` attribute is used. @@ -1058,11 +1061,7 @@ def add_figures( existing_figure = fig_name in self._figures if existing_figure and not overwrite: - raise ExperimentEntryExists( - f"A figure with the name {fig_name} for this experiment " - f"already exists. Specify overwrite=True if you " - f"want to overwrite it." - ) + # TODO remove any existing suffixes then generate new figure # figure_data = None if isinstance(figure, str): with open(figure, "rb") as file: From 7fe7d51d8ad68008733eb2e5a2227407c0a81eca Mon Sep 17 00:00:00 2001 From: Helena Zhang Date: Thu, 25 May 2023 17:57:39 -0400 Subject: [PATCH 4/9] update name format and `overwrite=False` behavior --- .../framework/experiment_data.py | 36 ++++++++++++------- .../update-figure-name-2db258c30ffe9912.yaml | 8 +++-- .../test_db_experiment_data.py | 25 ++++++++++--- 3 files changed, 49 insertions(+), 20 deletions(-) diff --git a/qiskit_experiments/framework/experiment_data.py b/qiskit_experiments/framework/experiment_data.py index 4e46dc12f6..bda34ef0cb 100644 --- a/qiskit_experiments/framework/experiment_data.py +++ b/qiskit_experiments/framework/experiment_data.py @@ -53,7 +53,6 @@ from qiskit_experiments.database_service.exceptions import ( ExperimentDataError, ExperimentEntryNotFound, - ExperimentEntryExists, ExperimentDataSaveFailed, ) @@ -1116,9 +1115,9 @@ def add_figures( figures: Paths of the figure files or figure data. figure_names: Names of the figures. If ``None``, use the figure file names, if given, or a generated name of the format ``experiment_type``, figure - index, first 5 elements of ``physical_qubits``, and first 8 digits of the experiment - ID connected by underscores. If `figures` is a list, then `figure_names` must also - be a list of the same length or ``None``. + index, first 5 elements of ``device_components``, and first 8 digits of the + experiment ID connected by underscores, such as ``T1_Q0_0123abcd.svg``. If `figures` + is a list, then `figure_names` must also be a list of the same length or ``None``. overwrite: Whether to overwrite the figure if one already exists with the same name. By default, overwrite is ``False`` and the figure will be renamed with an incrementing numerical suffix. For example, trying to save ``figure.svg`` when @@ -1128,11 +1127,9 @@ def add_figures( the ``auto-save`` attribute is used. Returns: - Figure names. + Figure names in SVG format. Raises: - ExperimentEntryExists: If the figure with the same name already exists, - and `overwrite=True` is not specified. ValueError: If an input parameter has an invalid value. """ if figure_names is not None and not isinstance(figure_names, list): @@ -1153,22 +1150,36 @@ def add_figures( elif not isinstance(figure, FigureData): fig_name = ( f"{self.experiment_type}_" - f"Fig-{len(self._figures)}_" - f'{"_".join(f"Q{i}" for i in self.metadata.get("physical_qubits", [])[:5])}_' - f"Exp-{self.experiment_id[:8]}.svg" + f'{"_".join(str(i) for i in self.metadata.get("device_components", [])[:5])}_' + f"{self.experiment_id[:8]}.svg" ) else: fig_name = figure.name else: fig_name = figure_names[idx] - if not fig_name.endswith(".svg"): LOG.info("File name %s does not have an SVG extension. A '.svg' is added.") fig_name += ".svg" existing_figure = fig_name in self._figures if existing_figure and not overwrite: - # TODO remove any existing suffixes then generate new figure + # Remove any existing suffixes then generate new figure name + fig_name_chunked = fig_name.rsplit("-", 1) + if len(fig_name_chunked) != 1: # Figure name already has a suffix + fig_name_prefix = fig_name_chunked[0] + try: + fig_name_suffix = int(fig_name_chunked[1].rsplit(".", 1)[0]) + except ValueError: # the suffix is not an int, add our own suffix + fig_name_prefix = fig_name.rsplit(".", 1)[0] + fig_name_suffix = 0 + else: + fig_name_prefix = fig_name.rsplit(".", 1)[0] + fig_name_suffix = 0 + fig_name = f"{fig_name_prefix}-{fig_name_suffix + 1}.svg" + while fig_name in self._figures: # Increment suffix until the name isn't taken + fig_name_suffix += 1 + fig_name = f"{fig_name_prefix}-{fig_name_suffix + 1}.svg" + # figure_data = None if isinstance(figure, str): with open(figure, "rb") as file: @@ -1181,6 +1192,7 @@ def add_figures( else: figure_metadata = { "qubits": self.metadata.get("physical_qubits"), + "device_components": self.metadata.get("device_components"), "experiment_type": self.experiment_type, } figure_data = FigureData(figure=figure, name=fig_name, metadata=figure_metadata) diff --git a/releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml b/releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml index 60c86b3a1d..800618b8c7 100644 --- a/releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml +++ b/releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml @@ -2,9 +2,11 @@ other: - | Figure names have been updated to include qubit indices up to the first five physical qubits in - the experiment, with format ``StandardRB_Fig-0_Q0_Q1_Q2_Q3_Q5_Exp-b4f1d8ad.svg``. For composite + the experiment, with format ``StandardRB_Q0_Q1_Q2_Q3_Q5_b4f1d8ad.svg``. For composite experiments where ``flatten_results`` is set to ``True``, the head of the figure name is now the class name of the experiment instead of ``ParallelExperiment`` or ``BatchExperiment``, such that - the figure name is the same when ``flatten_results`` is ``False``. + the figure name is the same when ``flatten_results`` is ``False``. The behavior when a figure + name is repeated and ``overwrite`` is ``False`` has changed from throwing an exception to + appending a numerical suffix to the figure name like ``StandardRB_Q0_Q1_Q2_Q3_Q5_b4f1d8ad-1.svg``. - | - Figure metadata now includes ``experiment_type``. + Figure metadata now includes ``experiment_type`` and ``device_components``. diff --git a/test/database_service/test_db_experiment_data.py b/test/database_service/test_db_experiment_data.py index 9f9cdd5ac4..a3cb0b7575 100644 --- a/test/database_service/test_db_experiment_data.py +++ b/test/database_service/test_db_experiment_data.py @@ -318,12 +318,26 @@ def test_add_figure_overwrite(self): exp_data = ExperimentData(backend=self.backend, experiment_type="qiskit_test") fn = exp_data.add_figures(hello_bytes) - with self.assertRaises(ExperimentEntryExists): - exp_data.add_figures(friend_bytes, fn) - exp_data.add_figures(friend_bytes, fn, overwrite=True) + # pylint: disable=no-member + fn_prefix = fn.rsplit(".", 1)[0] + + # Without overwrite on, the filename should have an incrementing suffix + self.assertEqual(exp_data.add_figures(friend_bytes, fn), f"{fn_prefix}-1.svg") + + self.assertEqual( + exp_data.add_figures([friend_bytes, friend_bytes], [fn, fn]), + [f"{fn_prefix}-2.svg", f"{fn_prefix}-3.svg"], + ) + + self.assertEqual(exp_data.add_figures(friend_bytes, fn, overwrite=True), fn) + self.assertEqual(friend_bytes, exp_data.figure(fn).figure) + self.assertEqual( + exp_data.add_figures(friend_bytes, f"{fn_prefix}-a.svg"), f"{fn_prefix}-a.svg" + ) + def test_add_figure_save(self): """Test saving a figure in the database.""" hello_bytes = str.encode("hello world") @@ -343,15 +357,16 @@ def test_add_figure_metadata(self): exp_data = ExperimentData( backend=self.backend, experiment_type="qiskit_test", - metadata={"physical_qubits": qubits}, + metadata={"physical_qubits": qubits, "device_components": list(map(Qubit, qubits))}, ) exp_data.add_figures(hello_bytes) exp_data.figure(0).metadata["foo"] = "bar" figure_data = exp_data.figure(0) self.assertEqual(figure_data.metadata["qubits"], qubits) + self.assertEqual(figure_data.metadata["device_components"], list(map(Qubit, qubits))) self.assertEqual(figure_data.metadata["foo"], "bar") - expected_name_prefix = "qiskit_test_Fig-0_Q0_Q1_Q2_Exp-" + expected_name_prefix = "qiskit_test_Q0_Q1_Q2_" self.assertEqual(figure_data.name[: len(expected_name_prefix)], expected_name_prefix) exp_data2 = ExperimentData( From 8c4fa937ea334b5f8a89533a020cff82e9d6ce25 Mon Sep 17 00:00:00 2001 From: Helena Zhang Date: Thu, 25 May 2023 18:03:48 -0400 Subject: [PATCH 5/9] lint --- test/database_service/test_db_experiment_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/database_service/test_db_experiment_data.py b/test/database_service/test_db_experiment_data.py index a3cb0b7575..2ad33175a8 100644 --- a/test/database_service/test_db_experiment_data.py +++ b/test/database_service/test_db_experiment_data.py @@ -336,7 +336,7 @@ def test_add_figure_overwrite(self): self.assertEqual( exp_data.add_figures(friend_bytes, f"{fn_prefix}-a.svg"), f"{fn_prefix}-a.svg" - ) + ) def test_add_figure_save(self): """Test saving a figure in the database.""" From 12bf6dd006f5fd823f78e5621bbc3e3179355df4 Mon Sep 17 00:00:00 2001 From: Helena Zhang Date: Thu, 25 May 2023 18:16:16 -0400 Subject: [PATCH 6/9] lint again --- test/database_service/test_db_experiment_data.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/database_service/test_db_experiment_data.py b/test/database_service/test_db_experiment_data.py index 2ad33175a8..42f333337f 100644 --- a/test/database_service/test_db_experiment_data.py +++ b/test/database_service/test_db_experiment_data.py @@ -40,7 +40,6 @@ from qiskit_experiments.database_service.exceptions import ( ExperimentDataError, ExperimentEntryNotFound, - ExperimentEntryExists, ) from qiskit_experiments.database_service.device_component import Qubit from qiskit_experiments.framework.experiment_data import ( From 7c949bbdb59d302e8bc5fb0718f73709227a1cae Mon Sep 17 00:00:00 2001 From: Helena Zhang Date: Wed, 26 Jul 2023 17:56:37 -0400 Subject: [PATCH 7/9] address review comments --- qiskit_experiments/framework/experiment_data.py | 16 +++++++++++++--- .../update-figure-name-2db258c30ffe9912.yaml | 2 +- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/qiskit_experiments/framework/experiment_data.py b/qiskit_experiments/framework/experiment_data.py index bda34ef0cb..f9a635b1db 100644 --- a/qiskit_experiments/framework/experiment_data.py +++ b/qiskit_experiments/framework/experiment_data.py @@ -1105,9 +1105,9 @@ def data( def add_figures( self, figures: Union[str, bytes, pyplot.Figure, list], - figure_names: Union[str, list] = None, - overwrite: bool = False, - save_figure: bool = None, + figure_names: Optional[Union[str, list]] = None, + overwrite: Optional[bool] = False, + save_figure: Optional[bool] = None, ) -> Union[str, List[str]]: """Add the experiment figure. @@ -1146,8 +1146,10 @@ def add_figures( for idx, figure in enumerate(figures): if figure_names is None: if isinstance(figure, str): + # figure is a file path, so we use it as the name fig_name = figure elif not isinstance(figure, FigureData): + # Generate a name in the form StandardRB_Q0_Q1_Q2_b4f1d8ad-1.svg fig_name = ( f"{self.experiment_type}_" f'{"_".join(str(i) for i in self.metadata.get("device_components", [])[:5])}_' @@ -1164,19 +1166,27 @@ def add_figures( existing_figure = fig_name in self._figures if existing_figure and not overwrite: # Remove any existing suffixes then generate new figure name + # StandardRB_Q0_Q1_Q2_b4f1d8ad.svg becomes StandardRB_Q0_Q1_Q2_b4f1d8ad fig_name_chunked = fig_name.rsplit("-", 1) if len(fig_name_chunked) != 1: # Figure name already has a suffix + # This extracts StandardRB_Q0_Q1_Q2_b4f1d8ad as the prefix from + # StandardRB_Q0_Q1_Q2_b4f1d8ad-1.svg fig_name_prefix = fig_name_chunked[0] try: fig_name_suffix = int(fig_name_chunked[1].rsplit(".", 1)[0]) except ValueError: # the suffix is not an int, add our own suffix + # my-custom-figure-name will be the prefix of my-custom-figure-name.svg fig_name_prefix = fig_name.rsplit(".", 1)[0] fig_name_suffix = 0 else: + # StandardRB_Q0_Q1_Q2_b4f1d8ad.svg has no hyphens so + # StandardRB_Q0_Q1_Q2_b4f1d8ad would be its prefix fig_name_prefix = fig_name.rsplit(".", 1)[0] fig_name_suffix = 0 fig_name = f"{fig_name_prefix}-{fig_name_suffix + 1}.svg" while fig_name in self._figures: # Increment suffix until the name isn't taken + # If StandardRB_Q0_Q1_Q2_b4f1d8ad-1.svg already exists, StandardRB_Q0_Q1_Q2_b4f1d8ad-2.svg + # will be the name of this figure fig_name_suffix += 1 fig_name = f"{fig_name_prefix}-{fig_name_suffix + 1}.svg" diff --git a/releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml b/releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml index 800618b8c7..4248c932c7 100644 --- a/releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml +++ b/releasenotes/notes/update-figure-name-2db258c30ffe9912.yaml @@ -1,7 +1,7 @@ --- other: - | - Figure names have been updated to include qubit indices up to the first five physical qubits in + Figure names have been updated to include qubit indices up to the first five device components in the experiment, with format ``StandardRB_Q0_Q1_Q2_Q3_Q5_b4f1d8ad.svg``. For composite experiments where ``flatten_results`` is set to ``True``, the head of the figure name is now the class name of the experiment instead of ``ParallelExperiment`` or ``BatchExperiment``, such that From 27780e38b6f5e48ad1c054c7a75e00f83cac0589 Mon Sep 17 00:00:00 2001 From: Helena Zhang Date: Wed, 26 Jul 2023 18:02:35 -0400 Subject: [PATCH 8/9] lint --- qiskit_experiments/framework/experiment_data.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qiskit_experiments/framework/experiment_data.py b/qiskit_experiments/framework/experiment_data.py index 7b4557cb1a..6f4480b5b1 100644 --- a/qiskit_experiments/framework/experiment_data.py +++ b/qiskit_experiments/framework/experiment_data.py @@ -1191,8 +1191,8 @@ def add_figures( fig_name_suffix = 0 fig_name = f"{fig_name_prefix}-{fig_name_suffix + 1}.svg" while fig_name in self._figures: # Increment suffix until the name isn't taken - # If StandardRB_Q0_Q1_Q2_b4f1d8ad-1.svg already exists, StandardRB_Q0_Q1_Q2_b4f1d8ad-2.svg - # will be the name of this figure + # If StandardRB_Q0_Q1_Q2_b4f1d8ad-1.svg already exists, + # StandardRB_Q0_Q1_Q2_b4f1d8ad-2.svg will be the name of this figure fig_name_suffix += 1 fig_name = f"{fig_name_prefix}-{fig_name_suffix + 1}.svg" From 1fa7ed3ce07acc80dd0bde80d0918b464b76522b Mon Sep 17 00:00:00 2001 From: Helena Zhang Date: Wed, 26 Jul 2023 18:06:27 -0400 Subject: [PATCH 9/9] update comments --- qiskit_experiments/framework/experiment_data.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qiskit_experiments/framework/experiment_data.py b/qiskit_experiments/framework/experiment_data.py index 6f4480b5b1..0aabf349f8 100644 --- a/qiskit_experiments/framework/experiment_data.py +++ b/qiskit_experiments/framework/experiment_data.py @@ -1152,7 +1152,7 @@ def add_figures( for idx, figure in enumerate(figures): if figure_names is None: if isinstance(figure, str): - # figure is a file path, so we use it as the name + # figure is a filename, so we use it as the name fig_name = figure elif not isinstance(figure, FigureData): # Generate a name in the form StandardRB_Q0_Q1_Q2_b4f1d8ad-1.svg @@ -1162,6 +1162,7 @@ def add_figures( f"{self.experiment_id[:8]}.svg" ) else: + # Keep the existing figure name if there is one fig_name = figure.name else: fig_name = figure_names[idx]