From 13c1c6316f17220225520344177ea459478b579e Mon Sep 17 00:00:00 2001 From: Bill Hlavacek Date: Thu, 23 Jul 2026 10:34:56 -0600 Subject: [PATCH] feat(bngsim): evaluate simulate/parameter_scan action args as expressions BNGL action arguments arrive as raw strings (`_parse_action_value` keeps scalars as text), so an arithmetic value like `t_end=>2*5` was handed to bare `float()`/`int()` and raised ValueError. Route every numeric argument in the simulate (`_prepare_simulate_run`) and parameter_scan/bifurcate (`_resolve_scan_settings`) handlers through `_eval_numeric` -- the same no-builtins safe evaluator `setParameter`/`setConcentration` already use -- so an expression resolves the way its literal equivalent does. Plain numbers are unchanged. Adds an e2e test (the expression form reproduces the literal run) plus a scan-settings test covering t_start/t_end/print_functions/reset_conc/seed. Co-Authored-By: Claude Opus 4.8 --- pybnf/bngsim_model/net_model.py | 31 ++++---- tests/test_bngsim_action_expressions.py | 101 ++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 15 deletions(-) create mode 100644 tests/test_bngsim_action_expressions.py diff --git a/pybnf/bngsim_model/net_model.py b/pybnf/bngsim_model/net_model.py index 045e14f9..60edf1a2 100644 --- a/pybnf/bngsim_model/net_model.py +++ b/pybnf/bngsim_model/net_model.py @@ -36,6 +36,7 @@ ) from .expressions import ( _build_safe_eval_namespace, + _eval_numeric, _eval_model_expression, _build_mutant_param_set, _parse_net_species_initializers, @@ -698,27 +699,27 @@ def _prepare_simulate_run(self, state, sim_params, model, action_index, timeout, sample_times = _resolve_sample_times(sim_params) # Gap 1: continue=>1 - continue_flag = bool(int(float(sim_params.get('continue', 0)))) + continue_flag = bool(int(_eval_numeric(str(sim_params.get('continue', 0))))) if continue_flag and 't_start' not in sim_params: t_start = state.current_time else: - t_start = float(sim_params.get('t_start', 0)) - t_end = float(sim_params.get('t_end', 100)) - n_steps = int(sim_params.get('n_steps', 100)) + t_start = _eval_numeric(str(sim_params.get('t_start', 0))) + t_end = _eval_numeric(str(sim_params.get('t_end', 100))) + n_steps = int(_eval_numeric(str(sim_params.get('n_steps', 100)))) suffix = sim_params.get('suffix', 'time_course') # Gap 4: print_functions - print_funcs = bool(int(float(sim_params.get('print_functions', 0)))) + print_funcs = bool(int(_eval_numeric(str(sim_params.get('print_functions', 0))))) # Gap 3: atol, rtol, seed run_kwargs = {} if 'atol' in sim_params: - run_kwargs['atol'] = float(sim_params['atol']) + run_kwargs['atol'] = _eval_numeric(str(sim_params['atol'])) if 'rtol' in sim_params: - run_kwargs['rtol'] = float(sim_params['rtol']) + run_kwargs['rtol'] = _eval_numeric(str(sim_params['rtol'])) explicit_seed = None if 'seed' in sim_params: - explicit_seed = int(float(sim_params['seed'])) + explicit_seed = int(_eval_numeric(str(sim_params['seed']))) if method in ('ssa', 'psa'): seed_value = self._resolve_action_seed( explicit_seed=explicit_seed, @@ -740,7 +741,7 @@ def _prepare_simulate_run(self, state, sim_params, model, action_index, timeout, # resetConcentrations between them). ``t_end`` remains the max-time bound for the # run if steady state is not reached within the window. Additive: no existing # simulate action emits ``steady_state``; only parameter_scan did (its own path). - if bool(int(float(sim_params.get('steady_state', 0)))): + if bool(int(_eval_numeric(str(sim_params.get('steady_state', 0))))): run_kwargs['steady_state'] = True # Gap 2: stop_if @@ -1047,12 +1048,12 @@ def _resolve_scan_settings(self, ps_params, is_bifurcate, action_index, timeout, """ concentration_overrides = concentration_overrides or {} param_name = ps_params.get('parameter', '') - t_start = float(ps_params.get('t_start', 0)) - t_end = float(ps_params.get('t_end', 100)) + t_start = _eval_numeric(str(ps_params.get('t_start', 0))) + t_end = _eval_numeric(str(ps_params.get('t_end', 100))) suffix = ps_params.get('suffix', 'param_scan') - use_ss = int(ps_params.get('steady_state', 0)) + use_ss = int(_eval_numeric(str(ps_params.get('steady_state', 0)))) ss_method = _normalize_ss_method(ps_params.get('ss_method')) - print_funcs = bool(int(float(ps_params.get('print_functions', 0)))) + print_funcs = bool(int(_eval_numeric(str(ps_params.get('print_functions', 0))))) method, poplevel = _normalize_action_method( ps_params.get('method', 'ode'), ps_params.get('poplevel'), @@ -1063,7 +1064,7 @@ def _resolve_scan_settings(self, ps_params, is_bifurcate, action_index, timeout, # produces distinct trajectories per the user's clarification. explicit_seed = None if 'seed' in ps_params: - explicit_seed = int(float(ps_params['seed'])) + explicit_seed = int(_eval_numeric(str(ps_params['seed']))) if method in ('ssa', 'psa'): scan_seed = self._resolve_action_seed( explicit_seed=explicit_seed, @@ -1086,7 +1087,7 @@ def _resolve_scan_settings(self, ps_params, is_bifurcate, action_index, timeout, if is_bifurcate: reset_conc = False else: - reset_conc = bool(int(float(ps_params.get('reset_conc', 1)))) + reset_conc = bool(int(_eval_numeric(str(ps_params.get('reset_conc', 1))))) # bifurcate is a continuation scan: it carries state between points # (reset_conc=False) to trace hysteresis/multistability. Independent diff --git a/tests/test_bngsim_action_expressions.py b/tests/test_bngsim_action_expressions.py new file mode 100644 index 00000000..44bf4f72 --- /dev/null +++ b/tests/test_bngsim_action_expressions.py @@ -0,0 +1,101 @@ +"""BNGL action arguments accept arithmetic expressions, not just bare numbers. + +``simulate(...)`` / ``parameter_scan(...)`` action arguments are parsed as raw +strings (``_parse_action_value`` preserves scalars as text), so a value written +as ``t_end=>2*5`` reaches the action handlers as the string ``"2*5"``. The +handlers evaluate every numeric argument through ``_eval_numeric`` -- the same +no-builtins safe evaluator ``setParameter``/``setConcentration`` already use -- +so an arithmetic expression resolves exactly the way its literal equivalent +would. These tests pin that behaviour: the expression form must reproduce the +literal form. (The prior bare-``float()``/``int()`` parsing raised ``ValueError`` +on ``"2*5"``.) +""" + +from pathlib import Path + +import numpy as np +import pytest + +import pybnf.bngsim_model as bngsim_model +from pybnf import pset + + +pytestmark = pytest.mark.bngsim + + +FIXTURES = Path(__file__).resolve().parent / 'bngl_files' + + +def _suffixes_for(actions): + """Extract the ``('simulate', )`` pairs from action lines.""" + pairs = [] + for a in actions: + i = a.find('suffix=>"') + if i == -1: + continue + j = a.find('"', i + len('suffix=>"')) + pairs.append(('simulate', a[i + len('suffix=>"'):j])) + return pairs + + +def _decay_model(actions): + """A BngsimModel over the committed exponential-decay .net fixture.""" + net_path = FIXTURES / 'e2e_ode_decay.net' + model = bngsim_model.BngsimModel( + net_path.stem, list(actions), _suffixes_for(actions), [], nf=str(net_path), + ) + model.param_set = pset.PSet([]) + return model + + +def test_simulate_action_args_accept_expressions(tmp_path): + """``t_end``/``n_steps`` written as arithmetic expressions (``2*5``, ``4*5``) + produce the identical run to their literal equivalents (``10``, ``20``). + + Two fresh models start from identical initial conditions, so any difference + would be the expression parsing -- and on the pre-change ``float()`` path the + expression form raised ``ValueError`` outright. + """ + literal = _decay_model( + ['simulate({method=>"ode",t_start=>0,t_end=>10,n_steps=>20,suffix=>"tc"})'], + ).execute(str(tmp_path), 'decay_literal', 60)['tc'] + expr = _decay_model( + ['simulate({method=>"ode",t_start=>0,t_end=>2*5,n_steps=>4*5,suffix=>"tc"})'], + ).execute(str(tmp_path), 'decay_expr', 60)['tc'] + + lit_t = literal.data[:, literal.cols['time']] + exp_t = expr.data[:, expr.cols['time']] + assert exp_t[-1] == pytest.approx(10.0) # t_end=>2*5 + assert len(exp_t) == len(lit_t) # n_steps=>4*5 + np.testing.assert_allclose(exp_t, lit_t) + + lit_s = literal.data[:, literal.cols['Stot']] + exp_s = expr.data[:, expr.cols['Stot']] + np.testing.assert_allclose(exp_s, lit_s) + + +def test_parameter_scan_action_args_accept_expressions(): + """The parameter_scan/bifurcate handler evaluates its numeric args through + ``_eval_numeric`` as well (``t_start``/``t_end``/``print_functions``/ + ``reset_conc``/``seed``). Resolving settings from expression-valued args must + land each on the expected number; bare ``float()``/``int()`` would raise on + ``"1+1"``. + """ + model = _decay_model([]) + ps_params = { + 'parameter': 'k', + 'par_scan_vals': ['1', '2', '3'], + 't_start': '1+1', # -> 2.0 + 't_end': '2*5', # -> 10.0 + 'print_functions': '1*1', # -> True + 'reset_conc': '0+1', # -> True + 'seed': '40+2', # -> 42 + 'suffix': 'scan', + } + settings = model._resolve_scan_settings(ps_params, False, 0, 60, None) + + assert settings.t_start == pytest.approx(2.0) + assert settings.t_end == pytest.approx(10.0) + assert settings.print_funcs is True + assert settings.reset_conc is True + assert settings.scan_seed == 42