Evaluate BNGL parameter expressions instead of dropping them

[wshlavacek]

A BNGL parameters block may give a parameter a value that is an expression over other parameters, such as kon koff/(KdNAV). That is ordinary BNGL rather than an edge case. Across 303 models drawn from the BioNetGen model collections, 1934 of 9323 parameter declarations are expression-valued.

Right now get_parameter_value raises NotImplementedError for those, and get_free_parameter_ids_with_values skips them without saying anything, so a PEtab problem built from a BNGL model quietly loses parameters. To reproduce on main:

from petab.v1.models.bngl_model import BnglModel, parse_bngl

text = """
begin parameters
  NA    6.022e23
  V     1e-12
  Kd    5.0
  koff  0.1
  kon   koff/(Kd*NA*V)
end parameters
"""
m = BnglModel(parse_bngl(text), model_id="demo")
print(list(m.get_parameter_ids()))
print(m.get_free_parameter_ids_with_values())

kon is listed as a parameter but is missing from the values, and nothing is said about it.

Resolving these needs no BNG2.pl and no reaction network, because a parameters block is only arithmetic over other parameters. This adds a parser and evaluator for that arithmetic and points BnglModel at it. Expressions are tokenized and parsed rather than passed to eval.

Getting the arithmetic right

BNGL arithmetic is not Python arithmetic, and the differences give a wrong number rather than an error. I checked each rule against BNG2.pl 2.9.3 by putting the expression in a parameters block and running writeNET({evaluate_expressions=>1}), which is the only export path that prints numbers instead of copying the source text back out. Three cases are easy to get wrong:

-2^2       is 4, because unary minus binds tighter than the power operator
2^3^2      is 64, because the power operator groups from the left
rint(2.5)  is 3, because rint is floor(x + 0.5)

The function table and the operator precedence follow Perl2/Expression.pm in the BioNetGen source. So _pi and _e take no arguments and are written _pi(), floor and ceil are rejected because they are commented out there as unsupported, and the comparison operators, the logical operators and if are supported because real models use them. A bare log is rejected, because BNGL's natural logarithm is ln and quietly accepting log would turn a typo into a plausible wrong number. TFUN is left out on purpose, because it reads a data file while a simulation runs and so is not a constant.

Partial resolution

get_free_parameter_ids_with_values now resolves what it can and warns about the rest by name. Failing the whole block on one bad definition would lose more than the current bug does. A real model can carry a construct we do not evaluate, and taking its other sixteen parameters down with it is a worse outcome than one silent drop.

Tests

The table of expressions and their expected values is checked from both sides. test_bng_verified_table pins the evaluator against it and needs no BioNetGen, so it runs in continuous integration. test_table_still_matches_bng2pl rebuilds the same values from a real BNG2.pl in a single run where one is available, and skips otherwise, so the table cannot go stale unnoticed if BioNetGen changes. I checked that the second test can actually fail by putting a wrong value in the table and watching it report the mismatch.

test_expression_valued_parameter_is_not_evaluated pinned the old behaviour, so I updated and renamed it.

The test suite passes apart from test_combine_archive and test_data_overview, which also fail on main here because of optional packages I do not have installed.

Credit

The evaluator started as a contribution from @arpitjain099 in lanl/PyBNF#673, which is where the parser design and many of the test cases come from. The BNG2.pl corrections and the partial resolution came out of review there.

[wshlavacek]

The Windows 3.14 job failed while downloading the test corpus rather than in
a test. It could not fetch Chattaraj_2021.bngl from the content delivery
network, reporting that the connection was forcibly closed by the remote
host. The other five jobs were cancelled by fail-fast rather than failing on
their own, so nothing here got as far as running a test.

This looks unrelated to the change. The same step failed on main on 26 August
at commit 847b16f, on a different file, and that same commit passed on the
days either side.

Locally the corpus fetches all 21 files with checksums verified, and
test_bngl_corpus.py passes.

Could someone re-run the job when you have a moment? I do not have the rights
to do it myself.

One thing worth looking at separately: the fetch step has no retry, so a
single transient network error fails the whole build.

[wshlavacek]

@dweindl this is ready for review whenever you have time. I do not have the
rights to assign a reviewer directly.

It will also need a CI re-run, for the corpus download reason in the comment
above.