BNGsim bridge silently falls back to legacy subprocess when action inspection fails under simulator='bngsim' (should raise)

[wshlavacek]

Summary

Under simulator='bngsim', the BNGsim routing bridge silently routes a model to the legacy BNG2.pl subprocess engine when it cannot inspect the model's actions with PyBioNetGen's Python parser (modelapi.bngmodel) — instead of raising. This violates the simulator='bngsim' contract ("use BNGsim, or error") and yields results that never ran through BNGsim, with no signal to the caller.

Where

bionetgen/core/tools/bngsim_bridge.py (observed on the PR #102 merge, 5109a46):

• _parse_bngl_actions_for_routing() returns None when bngmodel(path) raises (e.g. an unrecognized simulate argument).
• _classify_bngl_actions_for_bngsim(actions_items=None, ...) then returns BngsimRouteDecision(ROUTE_SUBPROCESS, "BNGL actions could not be inspected safely") — unconditionally, with no reference to simulator.
• classify_bngsim_route(..., simulator='bngsim') calls it and propagates that ROUTE_SUBPROCESS, so an explicit simulator='bngsim' request runs legacy BNG2.pl.

For the unavailable-BNGsim case classify_bngsim_route correctly returns ROUTE_ERROR; it's only the un-inspectable-actions case that silently downgrades the engine.

Why this is a problem

simulator='bngsim' means "require BNGsim, error if it can't run" — but here the run silently produces legacy output assumed to be BNGsim, with no exception and no warning, so the caller can't tell. (BNGsim itself can simulate these models — it works off the .net from BNG2.pl network generation; only the bridge's Python-side routing inspection fails.)

In a parity sweep over an 895-model BNGL corpus, 9 models silently produced legacy output this way; it was only caught by independently classifying every job's route via classify_bngsim_route.

Reproduce

A BNGL whose simulate carries an argument bngmodel rejects but BNG2.pl tolerates — real-world examples seen in published models: a typo'd atoll (for atol), abstol/reltol, or par_scan_vals placed on a simulate action:

# model.bngl — minimal:
begin model
begin parameters
 k 1.0
end parameters
begin species
 A() 100
 B() 0
end species
begin reaction rules
 A() -> B() k
end reaction rules
begin observables
 Molecules Atot A()
end observables
end model
generate_network({overwrite=>1})
simulate({method=>"ode",t_end=>10,n_steps=>100,atoll=>1e-8})

from bionetgen.core.tools import bngsim_bridge as br
fmt = br.detect_input_format("model.bngl")                 # 'bngl'
dec = br.classify_bngsim_route("model.bngl", fmt, simulator="bngsim", method=None)
print(dec.route, "|", dec.reason)
# subprocess | BNGL actions could not be inspected safely

bngmodel("model.bngl") raises BNGParseError: ... argument atoll not recognized for action simulate. (BNG2.pl silently ignores the unrecognized simulate arg and runs with default tolerance — which is itself why such typos survive in published models.)

Expected

Under simulator='bngsim', when action inspection fails, raise (return ROUTE_ERROR) and surface the underlying parse reason, so the caller learns the model couldn't be routed to BNGsim — and why. This deliberately keeps the strict parse: the trigger here is genuinely malformed BNGL, and raising is what surfaces it, rather than masking it behind a silently-degraded legacy run. (Under simulator='auto', the subprocess fallback is reasonable, but a one-time WARNING that a model couldn't be inspected for BNGsim routing would help.)

Suggested fix

Thread simulator into _classify_bngl_actions_for_bngsim (or handle it at the classify_bngsim_route call site, where simulator is already in scope): when actions_items is None, return ROUTE_ERROR if simulator == "bngsim" (propagating the parse reason), otherwise keep ROUTE_SUBPROCESS for auto/subprocess.