From 6f72b9c53c609adedf9a75d502d6bdc82a99c8fd Mon Sep 17 00:00:00 2001 From: Bill Hlavacek Date: Mon, 11 May 2026 14:00:09 -0600 Subject: [PATCH 1/2] Unify csimulator error handling --- bionetgen/core/exc.py | 19 ++++ bionetgen/simulator/csimulator.py | 88 ++++++++++-------- tests/test_csimulator_errors.py | 144 ++++++++++++++++++++++++++++++ 3 files changed, 214 insertions(+), 37 deletions(-) create mode 100644 tests/test_csimulator_errors.py diff --git a/bionetgen/core/exc.py b/bionetgen/core/exc.py index 11f5307f..699af0ca 100644 --- a/bionetgen/core/exc.py +++ b/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) diff --git a/bionetgen/simulator/csimulator.py b/bionetgen/simulator/csimulator.py index 4e7dd0d5..05313a65 100644 --- a/bionetgen/simulator/csimulator.py +++ b/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) diff --git a/tests/test_csimulator_errors.py b/tests/test_csimulator_errors.py new file mode 100644 index 00000000..c17968a8 --- /dev/null +++ b/tests/test_csimulator_errors.py @@ -0,0 +1,144 @@ +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( + "/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() From fd4c2815e4263d80b49e5b06528fc127046ea7c0 Mon Sep 17 00:00:00 2001 From: Bill Hlavacek Date: Mon, 11 May 2026 15:25:17 -0600 Subject: [PATCH 2/2] Fix csimulator test path expectation on Windows --- tests/test_csimulator_errors.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_csimulator_errors.py b/tests/test_csimulator_errors.py index c17968a8..3351e1c7 100644 --- a/tests/test_csimulator_errors.py +++ b/tests/test_csimulator_errors.py @@ -1,3 +1,4 @@ +import os from unittest import mock import pytest @@ -40,7 +41,7 @@ def fake_compile(self): mock.call("cvode_lib"), ] mock_wrapper.assert_called_once_with( - "/tmp/fake/libcsim.so", num_params=0, num_spec_init=0 + os.path.abspath("/tmp/fake/libcsim.so"), num_params=0, num_spec_init=0 )