Merge pull request #7240 from microsoft/copilot/fix-7051

Fix inferred parameters missing from dataset when explicitly added

Author: jenshnielsen

docs/changes/newsfragments/7240.improved

@@ -0,0 +1,9 @@
+The QCoDeS dataset now correctly captures data for inferred parameters when added via ``datasaver.add_result``. Previously these were discarded due to an oversight.
+Any parameters added in this way are available via ``DataSetProtocol.get_parameter_data`` but not exported to xarray or pandas.
+Future improvements to these parameters are planned and tracked `here <https://github.com/microsoft/Qcodes/issues/7060>`_.
+
+The property ``InterDependencies_.non_dependencies`` has been deprecated and replaced by ``InterDependencies_.top_level_parameters`` that correctly includes inferred parameters.
+The ``InterDependencies_`` object has gained two additional methods ``find_all_parameters_in_tree`` and ``all_parameters_in_tree_by_group``.
+
+The not documented but publicly importable functions ``qcodes.dataset.exporters.export_to_pandas.load_to_concatenated_dataframe`` and ``qcodes.dataset.exporters.export_to_pandas.load_to_dataframe_dict``
+now require an extra argument in the form of an ``InterDependencies_`` object. Users are discouraged from using anything not documented in `the QCoDeS API documentation <https://microsoft.github.io/Qcodes/api/index.html>`_ outside QCoDeS.

docs/examples/DataSet/Accessing-data-in-DataSet.ipynb

@@ -13,6 +13,7 @@
 from typing import TYPE_CHECKING, Any, Literal
 
 import numpy
+import numpy.typing as npt
 from tqdm.auto import trange
 
 import qcodes
@@ -851,8 +852,9 @@ def get_parameter_data(
         """
         if len(params) == 0:
             valid_param_names = [
-                ps.name for ps in self._rundescriber.interdeps.non_dependencies
+                ps.name for ps in self._rundescriber.interdeps.top_level_parameters
             ]
+
         else:
             valid_param_names = self._validate_parameters(*params)
         return get_parameter_data(
@@ -903,7 +905,7 @@ def to_pandas_dataframe_dict(
 
         """
         datadict = self.get_parameter_data(*params, start=start, end=end)
-        dfs_dict = load_to_dataframe_dict(datadict)
+        dfs_dict = load_to_dataframe_dict(datadict, self.description.interdeps)
         return dfs_dict
 
     def to_pandas_dataframe(
@@ -951,7 +953,7 @@ def to_pandas_dataframe(
 
         """
         datadict = self.get_parameter_data(*params, start=start, end=end)
-        return load_to_concatenated_dataframe(datadict)
+        return load_to_concatenated_dataframe(datadict, self.description.interdeps)
 
     def to_xarray_dataarray_dict(
         self,
@@ -1226,7 +1228,7 @@ def __repr__(self) -> str:
         return "\n".join(out)
 
     def _enqueue_results(
-        self, result_dict: Mapping[ParamSpecBase, numpy.ndarray]
+        self, result_dict: Mapping[ParamSpecBase, npt.NDArray]
     ) -> None:
         """
         Enqueue the results into self._results
@@ -1243,14 +1245,25 @@ def _enqueue_results(
         self._raise_if_not_writable()
         interdeps = self._rundescriber.interdeps
 
-        toplevel_params = set(interdeps.dependencies).intersection(set(result_dict))
+        result_parameters = set(result_dict.keys())
+        unused_results = result_parameters.copy()
+
+        toplevel_params = set(interdeps.top_level_parameters).intersection(
+            result_parameters
+        )
 
-        new_results: dict[str, dict[str, numpy.ndarray]] = {}
+        new_results: dict[str, dict[str, npt.NDArray]] = {}
 
         for toplevel_param in toplevel_params:
-            inff_params = set(interdeps.inferences.get(toplevel_param, ()))
-            deps_params = set(interdeps.dependencies.get(toplevel_param, ()))
-            all_params = inff_params.union(deps_params).union({toplevel_param})
+            # Transitively collect all parameters that are related to any parameter
+            # in the current tree, including parameters that dependencies are inferred from
+            all_params = interdeps.find_all_parameters_in_tree(toplevel_param)
+            # Only include parameters that are present in result_dict
+            # we keep track of results unused in any tree and raise a warning at the end
+            # if there are any
+            all_params = all_params.intersection(result_parameters)
+
+            unused_results = unused_results.difference(all_params)
 
             if self._in_memory_cache:
                 new_results[toplevel_param.name] = {}
@@ -1268,8 +1281,13 @@ def _enqueue_results(
             if toplevel_param.type == "array":
                 res_list = self._finalize_res_dict_array(result_dict, all_params)
             elif toplevel_param.type in ("numeric", "text", "complex"):
+                collected_params = all_params.copy()
+                collected_params.remove(toplevel_param)
+
                 res_list = self._finalize_res_dict_numeric_text_or_complex(
-                    result_dict, toplevel_param, inff_params, deps_params
+                    result_dict,
+                    toplevel_param,
+                    collected_params,
                 )
             else:
                 res_dict: dict[str, VALUE] = {
@@ -1278,18 +1296,12 @@ def _enqueue_results(
                 res_list = [res_dict]
             self._results += res_list
 
-        # Finally, handle standalone parameters
-
-        standalones = set(interdeps.standalones).intersection(set(result_dict))
-
-        if standalones:
-            stdln_dict = {st: result_dict[st] for st in standalones}
-            self._results += self._finalize_res_dict_standalones(stdln_dict)
-            if self._in_memory_cache:
-                for st in standalones:
-                    new_results[st.name] = {
-                        st.name: self._reshape_array_for_cache(st, result_dict[st])
-                    }
+        if len(unused_results) > 0:
+            log.warning(
+                f"Results for parameters {unused_results} were not added to the "
+                "DataSet because they are not part of the interdependencies. "
+                "This will be an error in a future version of QCoDeS. "
+            )
 
         if self._in_memory_cache:
             self.cache.add_data(new_results)
@@ -1328,10 +1340,9 @@ def reshaper(val: Any, ps: ParamSpecBase) -> VALUE:
 
     @staticmethod
     def _finalize_res_dict_numeric_text_or_complex(
-        result_dict: Mapping[ParamSpecBase, numpy.ndarray],
+        result_dict: Mapping[ParamSpecBase, npt.NDArray],
         toplevel_param: ParamSpecBase,
-        inff_params: set[ParamSpecBase],
-        deps_params: set[ParamSpecBase],
+        params: set[ParamSpecBase],
     ) -> list[dict[str, VALUE]]:
         """
         Make a res_dict in the format expected by DataSet.add_results out
@@ -1341,7 +1352,7 @@ def _finalize_res_dict_numeric_text_or_complex(
         """
 
         res_list: list[dict[str, VALUE]] = []
-        all_params = inff_params.union(deps_params).union({toplevel_param})
+        all_params = params.union({toplevel_param})
 
         t_map = {"numeric": float, "text": str, "complex": complex}
 
@@ -1352,21 +1363,16 @@ def _finalize_res_dict_numeric_text_or_complex(
         else:
             # We first massage all values into np.arrays of the same
             # shape
-            flat_results: dict[str, numpy.ndarray] = {}
+            flat_results: dict[str, npt.NDArray] = {}
 
             toplevel_val = result_dict[toplevel_param]
             flat_results[toplevel_param.name] = toplevel_val.ravel()
             N = len(flat_results[toplevel_param.name])
-            for dep in deps_params:
-                if result_dict[dep].shape == ():
-                    flat_results[dep.name] = numpy.repeat(result_dict[dep], N)
-                else:
-                    flat_results[dep.name] = result_dict[dep].ravel()
-            for inff in inff_params:
-                if numpy.shape(result_dict[inff]) == ():
-                    flat_results[inff.name] = numpy.repeat(result_dict[inff], N)
+            for param in params:
+                if result_dict[param].shape == ():
+                    flat_results[param.name] = numpy.repeat(result_dict[param], N)
                 else:
-                    flat_results[inff.name] = result_dict[inff].ravel()
+                    flat_results[param.name] = result_dict[param].ravel()
 
             # And then put everything into the list
 
@@ -1379,7 +1385,7 @@ def _finalize_res_dict_numeric_text_or_complex(
 
     @staticmethod
     def _finalize_res_dict_standalones(
-        result_dict: Mapping[ParamSpecBase, numpy.ndarray],
+        result_dict: Mapping[ParamSpecBase, npt.NDArray],
     ) -> list[dict[str, VALUE]]:
         """
         Massage all standalone parameters into the correct shape

src/qcodes/dataset/data_set.py

@@ -25,6 +25,7 @@
     import pandas as pd
     import xarray as xr
 
+    from qcodes.dataset.descriptions.dependencies import InterDependencies_
     from qcodes.dataset.descriptions.rundescriber import RunDescriber
     from qcodes.dataset.sqlite.connection import AtomicConnection
 
@@ -91,6 +92,30 @@ def data(self) -> ParameterData:
 
         return self._data
 
+    @staticmethod
+    def _empty_data_dict(
+        interdeps: InterDependencies_,
+    ) -> dict[str, dict[str, npt.NDArray]]:
+        """
+        Create an dictionary with empty numpy arrays as values
+        matching the expected output of ``DataSet``'s ``get_parameter_data`` /
+        ``cache.data`` so that the order of keys in the returned dictionary
+        is the same as the order of parameters in the interdependencies
+        in this class.
+        """
+
+        output: dict[str, dict[str, npt.NDArray]] = {}
+        for toplevel_param in interdeps.top_level_parameters:
+            toplevel_param, deps, infs = interdeps.all_parameters_in_tree_by_group(
+                toplevel_param
+            )
+
+            output[toplevel_param.name] = {}
+            params = [toplevel_param, *deps, *infs]
+            for param in params:
+                output[toplevel_param.name][param.name] = np.array([])
+        return output
+
     def prepare(self) -> None:
         """
         Set up the internal datastructure of the cache.
@@ -99,7 +124,7 @@ def prepare(self) -> None:
         """
 
         if self._data == {}:
-            self._data = self.rundescriber.interdeps._empty_data_dict()
+            self._data = self._empty_data_dict(self.rundescriber.interdeps)
         else:
             raise RuntimeError("Cannot prepare a cache that is not empty")
 
@@ -145,7 +170,7 @@ def to_pandas_dataframe_dict(self) -> dict[str, pd.DataFrame]:
 
         """
         data = self.data()
-        return load_to_dataframe_dict(data)
+        return load_to_dataframe_dict(data, self.rundescriber.interdeps)
 
     def to_pandas_dataframe(self) -> pd.DataFrame:
         """
@@ -158,7 +183,7 @@ def to_pandas_dataframe(self) -> pd.DataFrame:
 
         """
         data = self.data()
-        return load_to_concatenated_dataframe(data)
+        return load_to_concatenated_dataframe(data, self.rundescriber.interdeps)
 
     def to_xarray_dataarray_dict(
         self, *, use_multi_index: Literal["auto", "always", "never"] = "auto"
@@ -266,7 +291,7 @@ def append_shaped_parameter_data_to_existing_arrays(
         Updated write and read status, and the updated ``data``
 
     """
-    parameters = tuple(ps.name for ps in rundescriber.interdeps.non_dependencies)
+    parameters = tuple(ps.name for ps in rundescriber.interdeps.top_level_parameters)
     merged_data = {}
 
     updated_write_status = dict(write_status)

src/qcodes/dataset/data_set_cache.py

@@ -658,12 +658,24 @@ def _enqueue_results(
         self._raise_if_not_writable()
         interdeps = self._rundescriber.interdeps
 
-        toplevel_params = set(interdeps.dependencies).intersection(set(result_dict))
+        result_parameters = set(result_dict.keys())
+        toplevel_params = set(interdeps.top_level_parameters).intersection(
+            result_parameters
+        )
         new_results: dict[str, dict[str, npt.NDArray]] = {}
+
+        unused_results = result_parameters.copy()
+
         for toplevel_param in toplevel_params:
-            inff_params = set(interdeps.inferences.get(toplevel_param, ()))
-            deps_params = set(interdeps.dependencies.get(toplevel_param, ()))
-            all_params = inff_params.union(deps_params).union({toplevel_param})
+            # Transitively collect all parameters that are related to any parameter
+            # in the current tree, including parameters that dependencies are inferred from
+            all_params = interdeps.find_all_parameters_in_tree(toplevel_param)
+            # Only include parameters that are present in result_dict
+            # we keep track of results unused in any tree and raise a warning at the end
+            # if there are any
+            all_params = all_params.intersection(result_dict.keys())
+
+            unused_results = unused_results.difference(all_params)
 
             new_results[toplevel_param.name] = {}
             new_results[toplevel_param.name][toplevel_param.name] = (
@@ -677,15 +689,12 @@ def _enqueue_results(
                         self._reshape_array_for_cache(param, result_dict[param])
                     )
 
-        # Finally, handle standalone parameters
-
-        standalones = set(interdeps.standalones).intersection(set(result_dict))
-
-        if standalones:
-            for st in standalones:
-                new_results[st.name] = {
-                    st.name: self._reshape_array_for_cache(st, result_dict[st])
-                }
+        if len(unused_results) > 0:
+            log.warning(
+                f"Results for parameters {unused_results} were not added to the "
+                "DataSet because they are not part of the interdependencies. "
+                "This will be an error in a future version of QCoDeS. "
+            )
 
         self.cache.add_data(new_results)