Merge pull request #118 from RWTH-EBC/117-typeerror-is-wrongfully-raised [PYPI-RELEASE]

117 typeerror is wrongfully raised

Author: FWuellhorst

ebcpy/__init__.py

@@ -8,4 +8,4 @@
 from .optimization import Optimizer
 
 
-__version__ = '0.3.11'
+__version__ = '0.3.12'

ebcpy/simulationapi/__init__.py

@@ -292,32 +292,44 @@ def simulate(self,
             parameters = [{}]
         if isinstance(parameters, dict):
             parameters = [parameters]
+
+        if return_option not in ["time_series", "savepath", "last_point"]:
+            raise ValueError(f"Given return option '{return_option}' is not supported.")
+
         new_kwargs = {}
         kwargs["return_option"] = return_option  # Update with arg
+        n_simulations = len(parameters)
         # Handle special case for saving files:
-        if return_option == "savepath" and len(parameters) > 1:
+        if return_option == "savepath" and n_simulations > 1:
             savepath = kwargs.get("savepath", [])
             if isinstance(savepath, (str, os.PathLike)):
-                savepath = [savepath] * len(parameters)
+                savepath = [savepath] * n_simulations
             result_file_name = kwargs.get("result_file_name", [])
-            if (len(set(savepath)) != len(parameters) and
-                    len(set(result_file_name)) != len(parameters)):
-                raise TypeError(
+            if isinstance(result_file_name, str):
+                result_file_name = [result_file_name] * n_simulations
+            if len(savepath) != len(result_file_name):
+                raise ValueError("Given savepath and result_file_name "
+                                 "have not the same lenght.")
+            joined_save_paths = []
+            for _single_save_path, _single_result_name in zip(savepath, result_file_name):
+                joined_save_paths.append(os.path.join(_single_save_path, _single_result_name))
+            if len(set(joined_save_paths)) != n_simulations:
+                raise ValueError(
                     "Simulating multiple parameter set's on "
-                    "the same savepath will overwrite old "
-                    "results or even cause errors. "
-                    "Specify a result_file_name or savepath for each "
-                    "parameter combination"
+                    "the same combination of savepath and result_file_name "
+                    "will override results or even cause errors. "
+                    "Specify a unqiue result_file_name-savepath combination "
+                    "for each parameter combination"
                 )
         for key, value in kwargs.items():
             if isinstance(value, list):
-                if len(value) != len(parameters):
+                if len(value) != n_simulations:
                     raise ValueError(f"Mismatch in multiprocessing of "
-                                     f"given parameters ({len(parameters)}) "
+                                     f"given parameters ({n_simulations}) "
                                      f"and given {key} ({len(value)})")
                 new_kwargs[key] = value
             else:
-                new_kwargs[key] = [value] * len(parameters)
+                new_kwargs[key] = [value] * n_simulations
         kwargs = []
         for _idx, _parameters in enumerate(parameters):
             kwargs.append(
@@ -354,7 +366,7 @@ def simulate(self,
                 "return_option": _single_kwargs["return_option"],
                 **_single_kwargs
             }) for _single_kwargs in kwargs]
-        self.logger.info(f"Finished {len(parameters)} simulations on {self.n_cpu} processes in "
+        self.logger.info(f"Finished {n_simulations} simulations on {self.n_cpu} processes in "
                          f"{timedelta(seconds=int(time.time() - t_sim_start))}")
         if len(results) == 1:
             return results[0]

ebcpy/simulationapi/dymola_api.py

@@ -327,15 +327,24 @@ def simulate(self,
 
     def _single_simulation(self, kwargs):
         # Unpack kwargs
-        show_eventlog = kwargs.get("show_eventlog", False)
-        squeeze = kwargs.get("squeeze", True)
-        result_file_name = kwargs.get("result_file_name", 'resultFile')
-        parameters = kwargs.get("parameters")
-        return_option = kwargs.get("return_option")
-        model_names = kwargs.get("model_names")
-        inputs = kwargs.get("inputs", None)
-        fail_on_error = kwargs.get("fail_on_error", True)
-        structural_parameters = kwargs.get("structural_parameters", [])
+        show_eventlog = kwargs.pop("show_eventlog", False)
+        squeeze = kwargs.pop("squeeze", True)
+        result_file_name = kwargs.pop("result_file_name", 'resultFile')
+        parameters = kwargs.pop("parameters")
+        return_option = kwargs.pop("return_option")
+        model_names = kwargs.pop("model_names", None)
+        inputs = kwargs.pop("inputs", None)
+        fail_on_error = kwargs.pop("fail_on_error", True)
+        structural_parameters = kwargs.pop("structural_parameters", [])
+        table_name = kwargs.pop("table_name", None)
+        file_name = kwargs.pop("file_name", None)
+        savepath = kwargs.pop("savepath", None)
+        if kwargs:
+            self.logger.error(
+                "You passed the following kwargs which "
+                "are not part of the supported kwargs and "
+                "have thus no effect: %s.", " ,".join(list(kwargs.keys())))
+
 
         # Handle multiprocessing
         if self.use_mp:
@@ -422,10 +431,7 @@ def _single_simulation(self, kwargs):
         # Handle inputs
         if inputs is not None:
             # Unpack additional kwargs
-            try:
-                table_name = kwargs["table_name"]
-                file_name = kwargs["file_name"]
-            except KeyError as err:
+            if table_name is None or file_name is None:
                 raise KeyError("For inputs to be used by DymolaAPI.simulate, you "
                                "have to specify the 'table_name' and the 'file_name' "
                                "as keyword arguments of the function. These must match"
@@ -529,15 +535,17 @@ def _single_simulation(self, kwargs):
 
         if return_option == "savepath":
             _save_name_dsres = f"{result_file_name}.mat"
-            savepath = kwargs.pop("savepath", None)
             # Get the cd of the current dymola instance
             self.dymola.cd()
             # Get the value and convert it to a 100 % fitting str-path
             dymola_cd = str(pathlib.Path(self.dymola.getLastErrorLog().replace("\n", "")))
             if savepath is None or str(savepath) == dymola_cd:
                 return os.path.join(dymola_cd, _save_name_dsres)
             os.makedirs(savepath, exist_ok=True)
-            for filename in [_save_name_dsres, "dslog.txt", "dsfinal.txt"]:
+            for filename in [_save_name_dsres]:
+            # Copying dslogs and dsfinals can lead to errors,
+            # as the names are not unique
+            # for filename in [_save_name_dsres, "dslog.txt", "dsfinal.txt"]:
                 # Delete existing files
                 try:
                     os.remove(os.path.join(savepath, filename))
@@ -1028,7 +1036,7 @@ def get_dymola_path(dymola_install_dir, dymola_name=None):
         """
         if dymola_name is None:
             if "linux" in sys.platform:
-                dymola_name = "dymola.sh"
+                dymola_name = "dymola"
             elif "win" in sys.platform:
                 dymola_name = "Dymola.exe"
             else:

ebcpy/simulationapi/fmu.py

@@ -186,8 +186,17 @@ def _single_simulation(self, kwargs):
         # Unpack kwargs:
         parameters = kwargs.pop("parameters", None)
         return_option = kwargs.pop("return_option", "time_series")
-        inputs = kwargs.get("inputs", None)
-        fail_on_error = kwargs.get("fail_on_error", True)
+        inputs = kwargs.pop("inputs", None)
+        fail_on_error = kwargs.pop("fail_on_error", True)
+        result_file_name = kwargs.pop("result_file_name", "resultFile")
+        result_file_suffix = kwargs.pop("result_file_suffix", "csv")
+        parquet_engine = kwargs.pop('parquet_engine', 'pyarrow')
+        savepath = kwargs.pop("savepath", None)
+        if kwargs:
+            self.logger.error(
+                "You passed the following kwargs which "
+                "are not part of the supported kwargs and "
+                "have thus no effect: %s.", " ,".join(list(kwargs.keys())))
 
         if self.use_mp:
             if self._fmu_instance is None:
@@ -255,11 +264,6 @@ def _single_simulation(self, kwargs):
                             str(self.sim_setup.output_interval)[::-1].find('.'))
 
         if return_option == "savepath":
-            result_file_name = kwargs.get("result_file_name", "resultFile")
-            result_file_suffix = kwargs.get("result_file_suffix", "csv")
-            parquet_engine = kwargs.get('parquet_engine', 'pyarrow')
-            savepath = kwargs.get("savepath", None)
-
             if savepath is None:
                 savepath = self.cd
 

tests/test_simulationapi.py

@@ -94,10 +94,10 @@ def test_savepath_handling(self):
         _some_par = list(self.sim_api.parameters.keys())[0]
         pars = {_some_par: self.sim_api.parameters[_some_par].value}
         parameters = [pars for i in range(2)]
-        with self.assertRaises(TypeError):
+        with self.assertRaises(ValueError):
             res = self.sim_api.simulate(parameters=parameters,
                                         return_option='savepath')
-        with self.assertRaises(TypeError):
+        with self.assertRaises(ValueError):
             res = self.sim_api.simulate(parameters=parameters,
                                         return_option='savepath',
                                         result_file_name=["t", "t"],
@@ -183,7 +183,7 @@ def setUp(self) -> None:
 
         # Just for tests in the gitlab-ci:
         if "linux" in sys.platform:
-            dymola_exe_path = "/usr/local/bin64/dymola"
+            dymola_exe_path = "/usr/local/bin/dymola"
         else:
             dymola_exe_path = None
         try: