Avoid setting auto save triggering save for children in a composite e…

…xperiment (#1187)

### Summary

This PR changes the behavior of `ExprimentData.auto_save` such that
setting this field in the parent of a composite experiment does not
trigger the `save` method in the children experiments.

An unrelated change in this PR is setting a hard coded cap of 10 on the
numbers of workers that can be used in experiment saving.

### Details and comments

Currently setting `auto_save = True` triggers a save of the parent,
including the children. If in addition to that the `auto_save` field is
set to `True` in the children it triggers another save; this is usually
harmless, but it is an unexpected behavior which led to #1184.

This PR changes this. Children will save along with the parents' initial
save. This is done by adding a switch to save (which is on by default
but off when saving from the `auto_save` setter)

---------

Co-authored-by: Helena Zhang <[email protected]>

docs/howtos/cloud_service.rst

@@ -142,6 +142,11 @@ The :meth:`~.ExperimentData.auto_save` feature automatically saves changes to th
     You can view the experiment online at https://quantum-computing.ibm.com/experiments/cdaff3fa-f621-4915-a4d8-812d05d9a9ca
     <ExperimentData[T1], backend: ibmq_lima, status: ExperimentStatus.DONE, experiment_id: cdaff3fa-f621-4915-a4d8-812d05d9a9ca>
 
+Setting ``auto_save = True`` works by triggering :meth:`.ExperimentData.save`.
+
+When working with composite experiments, setting ``auto_save`` will propagate this
+setting to the child experiments.
+
 Deleting an experiment
 ~~~~~~~~~~~~~~~~~~~~~~
 

qiskit_experiments/framework/experiment_data.py

@@ -216,6 +216,7 @@ class ExperimentData:
     _json_decoder = ExperimentDecoder
 
     _metadata_filename = "metadata.json"
+    _max_workers_cap = 10
 
     def __init__(
         self,
@@ -717,8 +718,9 @@ def auto_save(self, save_val: bool) -> None:
         Args:
             save_val: Whether to do auto-save.
         """
-        if save_val is True and not self._auto_save:
-            self.save()
+        # children will be saved once we set auto_save for them
+        if save_val is True:
+            self.save(save_children=False)
         self._auto_save = save_val
         for res in self._analysis_results.values():
             # Setting private variable directly to avoid duplicate save. This
@@ -1175,6 +1177,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=fig_name)
+                figure = figure_data.figure
 
             else:
                 figure_metadata = {"qubits": self.metadata.get("physical_qubits")}
@@ -1485,15 +1488,20 @@ def _metadata_too_large(self):
         return sys.getsizeof(self.metadata) > 10000
 
     def save(
-        self, suppress_errors: bool = True, max_workers: int = 100, save_figures: bool = True
+        self,
+        suppress_errors: bool = True,
+        max_workers: int = 3,
+        save_figures: bool = True,
+        save_children: bool = True,
     ) -> None:
         """Save the experiment data to a database service.
 
         Args:
             suppress_errors: should the method catch exceptions (true) or
             pass them on, potentially aborting the experiemnt (false)
-            max_workers: Maximum number of concurrent worker threads
+            max_workers: Maximum number of concurrent worker threads (capped by 10)
             save_figures: Whether to save figures in the database or not
+            save_children: For composite experiments, whether to save children as well
 
         Raises:
             ExperimentDataSaveFailed: If no experiment database service
@@ -1518,7 +1526,13 @@ def save(
                 return
             else:
                 raise ExperimentDataSaveFailed("No service found")
-
+        if max_workers > self._max_workers_cap:
+            LOG.warning(
+                "max_workers cannot be larger than %s. Setting max_workers = %s now.",
+                self._max_workers_cap,
+                self._max_workers_cap,
+            )
+            max_workers = self._max_workers_cap
         self._save_experiment_metadata(suppress_errors=suppress_errors)
         if not self._created_in_db:
             LOG.warning("Could not save experiment metadata to DB, aborting experiment save")
@@ -1580,13 +1594,16 @@ def save(
                 f"https://quantum-computing.ibm.com/experiments/{self.experiment_id}"
             )
         # handle children, but without additional prints
-        for data in self._child_data.values():
-            original_verbose = data.verbose
-            data.verbose = False
-            data.save(
-                suppress_errors=suppress_errors, max_workers=max_workers, save_figures=save_figures
-            )
-            data.verbose = original_verbose
+        if save_children:
+            for data in self._child_data.values():
+                original_verbose = data.verbose
+                data.verbose = False
+                data.save(
+                    suppress_errors=suppress_errors,
+                    max_workers=max_workers,
+                    save_figures=save_figures,
+                )
+                data.verbose = original_verbose
 
     def jobs(self) -> List[Job]:
         """Return a list of jobs for the experiment"""
@@ -1714,7 +1731,6 @@ def block_for_results(self, timeout: Optional[float] = None) -> "ExperimentData"
 
         # Wait for futures
         self._wait_for_futures(job_futs + analysis_futs, name="jobs and analysis", timeout=timeout)
-
         # Clean up done job futures
         num_jobs = len(job_ids)
         for jid, fut in zip(job_ids, job_futs):

releasenotes/notes/experiment_data_save_bugfixes-48fe62bf2bfe38b8.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    :meth:`ExperimentData.auto_save` setter no longer saves sub-experiments twice.
+  - |
+    :meth:`ExperimentData.save` now handles correctly figures in sub-experiments when `flatten_results=True`

requirements.txt

@@ -1,7 +1,7 @@
 numpy>=1.17
 scipy>=1.4
 qiskit-terra>=0.24
-qiskit-ibm-experiment>=0.3.1
+qiskit-ibm-experiment>=0.3.3
 matplotlib>=3.4
 uncertainties
 lmfit

test/fake_experiment.py

@@ -13,6 +13,7 @@
 """A FakeExperiment for testing."""
 
 import numpy as np
+from matplotlib.figure import Figure as MatplotlibFigure
 from qiskit_experiments.framework import BaseExperiment, BaseAnalysis, Options, AnalysisResultData
 
 
@@ -25,13 +26,17 @@ def __init__(self, **kwargs):
         super().__init__()
         self._kwargs = kwargs
 
-    def _run_analysis(self, experiment_data, **options):
-        seed = options.get("seed", None)
+    def _run_analysis(self, experiment_data):
+        seed = self.options.get("seed", None)
         rng = np.random.default_rng(seed=seed)
         analysis_results = [
             AnalysisResultData(f"result_{i}", value) for i, value in enumerate(rng.random(3))
         ]
-        return analysis_results, None
+        figures = None
+        add_figures = self.options.get("add_figures", False)
+        if add_figures:
+            figures = [MatplotlibFigure()]
+        return analysis_results, figures
 
 
 class FakeExperiment(BaseExperiment):

test/framework/test_composite.py

@@ -17,6 +17,7 @@
 
 from test.fake_experiment import FakeExperiment, FakeAnalysis
 from test.base import QiskitExperimentsTestCase
+from unittest import mock
 from ddt import ddt, data
 
 from qiskit import QuantumCircuit, Aer
@@ -365,6 +366,36 @@ def test_composite_tags(self):
         self.assertEqual(sorted(data1.tags), ["c", "d"])
         self.assertEqual(sorted(data2.tags), ["c", "d"])
 
+    def test_composite_figures(self):
+        """
+        Test adding figures from composite experiments
+        """
+        exp1 = FakeExperiment([0, 2])
+        exp2 = FakeExperiment([1, 3])
+        exp1.analysis.set_options(add_figures=True)
+        exp2.analysis.set_options(add_figures=True)
+        par_exp = BatchExperiment([exp1, exp2], flatten_results=False)
+        expdata = par_exp.run(FakeBackend())
+        self.assertExperimentDone(expdata)
+        expdata.service = IBMExperimentService(local=True, local_save=False)
+        expdata.auto_save = True
+        par_exp.analysis.run(expdata)
+        self.assertExperimentDone(expdata)
+
+    def test_composite_auto_save(self):
+        """
+        Test setting autosave when using composite experiments
+        """
+        service = mock.create_autospec(IBMExperimentService, instance=True)
+        exp1 = FakeExperiment([0, 2])
+        exp2 = FakeExperiment([1, 3])
+        par_exp = BatchExperiment([exp1, exp2], flatten_results=False)
+        expdata = par_exp.run(FakeBackend())
+        expdata.service = service
+        self.assertExperimentDone(expdata)
+        expdata.auto_save = True
+        self.assertEqual(service.create_or_update_experiment.call_count, 3)
+
     def test_composite_subexp_data(self):
         """
         Verify that sub-experiment data of parallel and batch