Port the BNGL parameter-expression evaluator to libpetab-python's BnglModel, then delete the staging copy

[wshlavacek]

pybnf/petab/_bngl_expr.py evaluates the expressions a BioNetGen parameters block can give a parameter, such as kon koff/Kd. It was written for PyBNF's own BnglModel adapter, which #591 retired once petab 0.9.0 shipped a BioNetGen loader upstream. Nothing in pybnf/ has called it since. The file stays in the tree only as the staging copy for the upstream port, which is why it is stdlib-only and imports nothing from the rest of PyBNF.

Status: the port is written and open as PEtab-dev/libpetab-python#517, waiting on review. It has two commits, the port itself and then a restriction made after a maintainer objected.

The goal changed along the way, so it is worth stating plainly. This issue was written to get every expression-valued parameter evaluated. What will land evaluates only a parameter whose right hand side refers to no other parameter, and deliberately declines to give a value for the rest, because a PEtab parameter table may override or estimate the parameters a derived value is computed from. Checked against BNG2.pl rather than argued: set kon to the value its file defaults imply, 0.02, and then set koff to a fitted 10, and BNG2.pl writes "kon 0.02 # Constant", so the expression is gone and the number is wrong by a factor of a hundred. Leave kon alone and the same model writes "kon 2 # ConstantExpression".

Two corrections to the original description of this issue.

It said the gap loses the parameter "from petab's parameter-table checks". That is wrong. No lint task in petab calls either accessor, in version 1 or version 2. The two places that do are petab.v1.parameters.create_parameter_df, which fills a parameter table's nominal values, and petab.v1.parameter_mapping, which simulators use to bind parameters per condition. PyBNF itself calls neither, so nothing here is waiting on this.

It also understated one thing. On released petab 0.9.0 get_parameter_value raises NotImplementedError for any parameter whose value is an expression, and create_parameter_df catches only ValueError, so the error escapes that helper instead of leaving the nominal value empty. The upstream change raises ValueError and puts the class back on the contract its own base class documents.

What to do when a petab release below 1.0 carries the port:

Delete pybnf/petab/_bngl_expr.py and tests/test_petab_bngl_expr.py. The upstream evaluator is equivalent, and the table of expressions with the values BNG2.pl computes for them travelled with it, including the live cross-check against a real BNG2.pl.

That deletion is conditional on a question still open in the upstream pull request, so check the answer before deleting anything. The evaluator has two halves. evaluate_bngl_expression evaluates a single expression, and evaluate_bngl_parameters with its partial variant resolve a whole parameters block in dependency order. BnglModel no longer calls the second half, because it no longer evaluates a parameter defined in terms of other parameters, so the maintainer was asked whether those functions stay in petab or whether the pull request should be cut back to the first half and bring them back with PEtab-dev/libpetab-python#518. If they stay, delete our copy as above. If they are cut, dependency-order resolution exists only in pybnf/petab/_bngl_expr.py, and deleting it would throw away the only implementation of it we have, in which case the file stays until #796 gives it a home.

Do not bump the petab version floor, which the original text asked for. Nothing in PyBNF calls the two accessors, so a higher floor would buy no behaviour and would strand anyone on petab 0.9.0 for nothing.

Update docs/adr/0026, which describes the old contract in several places, including the accessor table at line 66, the grammar note at line 151, the test contract at line 225, the argument from 20.8% of parameter declarations at lines 246 to 252, and the closing paragraph at line 302.

There is no version pin to watch by hand. PyBNF declares a range, petab>=0.9,<1, and continuous integration resolves it fresh on every run, so a release carrying the port arrives without an edit here. The contract itself is now pinned by a test on main, added in #794, so a change in what petab reports for a derived parameter will be caught rather than absorbed silently.

Related but separate: #796 proposes moving the reader and the evaluator into a package shared with petab. That is a later step, gated on a different answer, and it does not block the deletion above.

[wshlavacek]

Correction to the description above, after tracing every caller of the two methods in petab 0.9.0 and in PyBNF: this gap does not affect PyBNF, and it does not affect petab's validator either.

• PyBNF calls neither method. The importer reads the model through PyBNF's own reader and uses only parameter names (for the observable-formula namespace and the shadow check). The exporter takes nominal values from the conf's free and fixed parameters and carries the model file verbatim, so BNG2.pl evaluates any expression at simulation time. Neither path ever asked the adapter for a value, before or after Evaluate BNGL parameter expressions instead of dropping them #673.
• petab v2 lint calls neither method. No lint task touches get_parameter_value or get_free_parameter_ids_with_values, so petablint on a BNGL problem with expression-valued parameters is unaffected. The sentence in the description about losing the parameter "from petab's parameter-table checks" is wrong.
• Where it does bite: two petab v1 helpers. petab.v1.parameters.create_parameter_df, which fills a parameter table's nominal values from the model, and petab.v1.parameter_mapping, which simulators such as AMICI or pyPESTO use to bind model parameters per condition. In both, an expression-valued BNGL parameter is silently skipped.

So the port is an improvement for petab users of BNGL models and for any future simulator that consumes a BNGL PEtab problem through petab's parameter mapping, not something PyBNF is waiting on. A PyBNF-side fix would mean re-registering a subclass into petab's model factory, which is the register_bngl() shim #591 just retired, and it would help only code that imports PyBNF first. Wrong layer.

Decision: keep this open as low priority. The evaluator stays in the tree as the staging copy for the upstream port; if the port is ever dropped instead, delete pybnf/petab/_bngl_expr.py and tests/test_petab_bngl_expr.py and close this.