Unify csimulator error handling

bionetgen/core/exc.py

@@ -93,3 +93,22 @@ def __init__(
         self.model = model
         self.message = message
         super().__init__(self.message)
+
+
+class BNGFormatError(BNGError):
+    """Error related to input file format detection or disambiguation."""
+
+    def __init__(self, path=None, message=None):
+        self.path = path
+        if message is None:
+            message = f"Could not determine the format of input file: {path}"
+        self.message = message
+        super().__init__(self.message)
+
+
+class BNGSimError(BNGError):
+    """Error related to BNGsim simulation."""
+
+    def __init__(self, message="There was an issue running BNGsim simulation"):
+        self.message = message
+        super().__init__(self.message)

bionetgen/simulator/csimulator.py

@@ -3,7 +3,8 @@
 
 from .bngsimulator import BNGSimulator
 from bionetgen.main import BioNetGen
-from bionetgen.core.exc import BNGCompileError
+from bionetgen.core.exc import BNGCompileError, BNGFormatError, BNGSimError
+from bionetgen.core.utils.logging import BNGLogger
 
 
 def _new_ccompiler():
@@ -31,6 +32,7 @@ def _new_ccompiler():
 app.setup()
 conf = app.config["bionetgen"]
 def_bng_path = conf["bngpath"]
+logger = BNGLogger()
 
 
 class RESULT(ctypes.Structure):
@@ -161,7 +163,10 @@ class CSimulator(BNGSimulator):
     def __init__(self, model_file, generate_network=False):
         # check cvode library paths
         if (conf.get("cvode_include") is None) or (conf.get("cvode_lib") is None):
-            print("CVODE include and library paths are not set, compilation won't work")
+            logger.warning(
+                "CVODE include and library paths are not set, compilation won't work",
+                loc=f"{__file__} : CSimulator.__init__()",
+            )
         # let's load the model first
         if isinstance(model_file, str):
             # load model file
@@ -182,7 +187,12 @@ def __init__(self, model_file, generate_network=False):
                 )
             os.chdir(cd)
         else:
-            print(f"model format not recognized: {model_file}")
+            msg = (
+                "CSimulator model input must be a BNGL path or bngmodel instance, "
+                f"got {type(model_file).__name__}"
+            )
+            logger.error(msg, loc=f"{__file__} : CSimulator.__init__()")
+            raise BNGFormatError(message=msg)
         # set compiler
         self.compiler = _new_ccompiler()
         self.compiler.add_include_dir(conf.get("cvode_include"))
@@ -223,6 +233,33 @@ def compile_shared_lib(self):
         lib_file = f"lib{self.model.model_name}_cvode_py.so"
         self.lib_file = os.path.abspath(lib_file)
 
+    def _get_numeric_parameter_values(self):
+        valid_params = []
+        for pname in self.model.parameters:
+            if pname.startswith("_"):
+                continue
+            val = self.model.parameters[pname]
+            try:
+                valid_params.append(float(val.expr))
+            except (AttributeError, TypeError, ValueError):
+                continue
+        return valid_params
+
+    def _resolve_species_count(self, spc_name):
+        count_value = self.model.species[spc_name].count
+        try:
+            return float(count_value)
+        except (TypeError, ValueError):
+            try:
+                return float(self.model.parameters[count_value].value)
+            except (AttributeError, KeyError, TypeError, ValueError) as exc:
+                msg = (
+                    f"Could not resolve initial species value for '{spc_name}' "
+                    f"from '{count_value}'"
+                )
+                logger.error(msg, loc=f"{__file__} : CSimulator.simulate()")
+                raise BNGSimError(msg) from exc
+
     @property
     def simulator(self):
         """
@@ -237,50 +274,27 @@ def simulator(self):
     def simulator(self, lib_file):
         # use CSimWrapper under the hood
         try:
-            valid_params = []
-            for pname in self.model.parameters:
-                if pname.startswith("_"):
-                    continue
-                val = self.model.parameters[pname]
-                try:
-                    ftry = float(val.expr)
-                    valid_params.append(ftry)
-                except:
-                    pass
+            valid_params = self._get_numeric_parameter_values()
             n_param = len(valid_params)
             self._simulator = CSimWrapper(
                 os.path.abspath(lib_file),
                 num_params=n_param,
                 num_spec_init=len(self.model.species),
             )
-        except:
-            raise BNGCompileError(self.model)
+        except (AttributeError, KeyError, OSError, TypeError, ValueError) as exc:
+            logger.error(
+                f"Failed to initialize C simulator wrapper: {exc}",
+                loc=f"{__file__} : CSimulator.simulator.setter()",
+            )
+            raise BNGCompileError(self.model) from exc
 
     def simulate(self, t_start=0, t_end=10, n_steps=10):
         # set parameters and initial species values
-        spcs = []
-        for spc_name in self.model.species:
-            try:
-                count = float(self.model.species[spc_name].count)
-                spcs.append(count)
-            except:
-                p_name = self.model.species[spc_name].count
-                count = float(self.model.parameters[p_name].value)
-                spcs.append(count)
+        spcs = [
+            self._resolve_species_count(spc_name) for spc_name in self.model.species
+        ]
         self.simulator.set_species_init(spcs)
-        params = []
-        for pname in self.model.parameters:
-            if pname.startswith("_"):
-                continue
-            try:
-                val = self.model.parameters[pname]
-                ftry = float(val.expr)
-                params.append(ftry)
-            except:
-                pass
-
-        # params = list(filter(lambda x: not x.startswith("_"), self.model.parameters))
-        # params = [float(self.model.parameters[p].value) for p in params]
+        params = self._get_numeric_parameter_values()
         self.simulator.set_parameters(params)
         # now that we have CSimWrapper setup correctly, run the simulation
         timepoints, obs_all, spcs_all = self.simulator.simulate(t_start, t_end, n_steps)

tests/test_csimulator_errors.py

@@ -0,0 +1,145 @@
+import os
+from unittest import mock
+
+import pytest
+
+
+def test_csimulator_init_logs_missing_cvode_paths():
+    from bionetgen.simulator import csimulator as csim_module
+
+    fake_model = mock.MagicMock()
+    fake_model.parameters = {}
+    fake_model.species = {}
+    fake_compiler = mock.MagicMock()
+    mock_conf_get = mock.MagicMock(side_effect=lambda key: None)
+
+    def fake_compile(self):
+        self.lib_file = "/tmp/fake/libcsim.so"
+
+    with mock.patch.object(csim_module.conf, "get", mock_conf_get), mock.patch.object(
+        csim_module, "logger"
+    ) as mock_logger, mock.patch.object(
+        csim_module.bionetgen, "bngmodel", return_value=fake_model
+    ), mock.patch.object(
+        csim_module, "_new_ccompiler", return_value=fake_compiler
+    ), mock.patch.object(
+        csim_module.CSimulator, "compile_shared_lib", fake_compile
+    ), mock.patch.object(
+        csim_module, "CSimWrapper"
+    ) as mock_wrapper:
+        csim_module.CSimulator("/fake/model.bngl")
+
+    mock_logger.warning.assert_called_once()
+    warning_args, warning_kwargs = mock_logger.warning.call_args
+    assert "CVODE include and library paths are not set" in warning_args[0]
+    assert "CSimulator.__init__()" in warning_kwargs["loc"]
+    fake_compiler.add_include_dir.assert_called_once_with(None)
+    fake_compiler.add_library_dir.assert_called_once_with(None)
+    assert mock_conf_get.call_args_list == [
+        mock.call("cvode_include"),
+        mock.call("cvode_include"),
+        mock.call("cvode_lib"),
+    ]
+    mock_wrapper.assert_called_once_with(
+        os.path.abspath("/tmp/fake/libcsim.so"), num_params=0, num_spec_init=0
+    )
+
+
+def test_csimulator_init_invalid_model_type_raises_bng_format_error():
+    from bionetgen.core.exc import BNGFormatError
+    from bionetgen.simulator import csimulator as csim_module
+
+    mock_conf_get = mock.MagicMock(
+        side_effect=lambda key: {
+            "cvode_include": "/tmp/include",
+            "cvode_lib": "/tmp/lib",
+        }[key]
+    )
+
+    with mock.patch.object(csim_module.conf, "get", mock_conf_get), mock.patch.object(
+        csim_module, "logger"
+    ) as mock_logger:
+        with pytest.raises(
+            BNGFormatError,
+            match="CSimulator model input must be a BNGL path or bngmodel instance",
+        ):
+            csim_module.CSimulator(123)
+
+    mock_logger.error.assert_called_once()
+    error_args, error_kwargs = mock_logger.error.call_args
+    assert "got int" in error_args[0]
+    assert "CSimulator.__init__()" in error_kwargs["loc"]
+    assert mock_conf_get.call_args_list == [
+        mock.call("cvode_include"),
+        mock.call("cvode_lib"),
+    ]
+
+
+def test_csimulator_simulator_setter_raises_bng_compile_error():
+    from bionetgen.core.exc import BNGCompileError
+    from bionetgen.simulator import csimulator as csim_module
+
+    sim = csim_module.CSimulator.__new__(csim_module.CSimulator)
+    sim.model = mock.MagicMock()
+    sim.model.parameters = {"k1": mock.MagicMock(expr="0.1")}
+    sim.model.species = {"A": mock.MagicMock(count="1")}
+
+    with mock.patch.object(
+        csim_module, "CSimWrapper", side_effect=OSError("boom")
+    ), mock.patch.object(csim_module, "logger") as mock_logger:
+        with pytest.raises(BNGCompileError):
+            sim.simulator = "/fake/lib.so"
+
+    mock_logger.error.assert_called_once()
+    error_args, error_kwargs = mock_logger.error.call_args
+    assert "Failed to initialize C simulator wrapper: boom" in error_args[0]
+    assert "CSimulator.simulator.setter()" in error_kwargs["loc"]
+
+
+def test_csimulator_simulate_resolves_species_parameter_counts():
+    from bionetgen.simulator.csimulator import CSimulator
+
+    sim = CSimulator.__new__(CSimulator)
+    sim.model = mock.MagicMock()
+    sim.model.species = {
+        "A": mock.MagicMock(count="5"),
+        "B": mock.MagicMock(count="k_init"),
+    }
+    sim.model.parameters = {
+        "k_init": mock.MagicMock(value="7.5", expr="7.5"),
+        "k_rate": mock.MagicMock(value="0.1", expr="0.1"),
+        "_hidden": mock.MagicMock(value="9.9", expr="9.9"),
+        "expr_only": mock.MagicMock(value="unused", expr="k_rate * 2"),
+    }
+    sim._simulator = mock.MagicMock()
+    sim._simulator.simulate.return_value = ("t", "obs", "spcs")
+
+    result = sim.simulate(t_start=1, t_end=4, n_steps=3)
+
+    sim._simulator.set_species_init.assert_called_once_with([5.0, 7.5])
+    sim._simulator.set_parameters.assert_called_once_with([7.5, 0.1])
+    sim._simulator.simulate.assert_called_once_with(1, 4, 3)
+    assert result == ("t", "obs", "spcs")
+
+
+def test_csimulator_simulate_invalid_species_reference_raises_bng_sim_error():
+    from bionetgen.core.exc import BNGSimError
+    from bionetgen.simulator import csimulator as csim_module
+
+    sim = csim_module.CSimulator.__new__(csim_module.CSimulator)
+    sim.model = mock.MagicMock()
+    sim.model.species = {"A": mock.MagicMock(count="missing_param")}
+    sim.model.parameters = {}
+    sim._simulator = mock.MagicMock()
+
+    with mock.patch.object(csim_module, "logger") as mock_logger:
+        with pytest.raises(
+            BNGSimError, match="Could not resolve initial species value for 'A'"
+        ):
+            sim.simulate()
+
+    mock_logger.error.assert_called_once()
+    error_args, error_kwargs = mock_logger.error.call_args
+    assert "missing_param" in error_args[0]
+    assert "CSimulator.simulate()" in error_kwargs["loc"]
+    sim._simulator.set_species_init.assert_not_called()