Add support for BNGL models (cont.) - #508
Open
dweindl wants to merge 12 commits into
Open
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #508 +/- ##
==========================================
+ Coverage 75.75% 76.01% +0.25%
==========================================
Files 65 67 +2
Lines 7357 7524 +167
Branches 1323 1341 +18
==========================================
+ Hits 5573 5719 +146
- Misses 1285 1302 +17
- Partials 499 503 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add a BnglModel loader (a peer of PySBModel/SbmlModel) so that a `language: bngl` PEtab problem loads and validates via petablint / Problem.from_yaml at the model level. See PEtab-dev/PEtab#436. - petab/v1/models/bngl_model.py: BnglModel backed by a small, dependency-free BNGL block reader (parse_bngl). Introspection only; is_valid shells out to `BNG2.pl --check` when a BNG backend is locatable and falls back to True otherwise (mirroring how the SBML loader always validates because libsbml is always present). - Register `bngl` in known_model_types and add a branch to model_factory; v2 picks it up via the existing re-exports (+ a v2 shim module). - tests/v1/test_model_bngl.py and a minimal BNGL fixture: ABC unit tests plus a full Problem.from_yaml validation oracle covering the model-cross checks. # Conflicts: # petab/v1/models/__init__.py
Re-sync with PyBNF's sibling reader (pybnf/petab/_bngl.py, ADR-0026 / lanl/PyBNF#437): - Block aliases: `begin molecules` / `begin species` / `begin rules` now open the same blocks as `molecule types` / `seed species` / `reaction rules` (_block_lines consults a per-canonical-name alias table), per the BNGL grammar reference (BioNetGen Perl2/; BNG_vscode_extension docs/bngl-grammar.md). - Seed-species `$` clamp: `SeedSpeciesDefn = ["$"], Species, ...` -- the `$` fixed-concentration marker is stripped so `$counter() 10` enumerates the state variable `counter()`, keeping is_state_variable correct under the clamp. Adds grammar-hardening tests (alias parsing, `$`-clamp stripping, no cross-block shadowing, the is_state_variable seam) that double as the drift anchor against PyBNF's reader. ruff check + format clean; 19 passed. Refs: PEtab-dev/PEtab#436.
A trailing `\` (BNGL line continuation) splits one logical declaration across physical lines. The block scanner processed physical lines, so a continued parameter/function/observable was truncated at the `\` (e.g. `k = \` read as the value `\`). Add _logical_lines() mirroring BNG2.pl's readFile (Perl2/BNGModel.pm): strip the comment first, then while a line ends with `\` drop it and concatenate the next comment-stripped physical line directly (no space, so `1e\`+`3` -> `1e3`). Surfaced by the bng_parity corpus (895 community BNGL models): 252 use line continuation, incl. inside enumerated blocks (functions, observables, parameters, seed species). Adds continuation + backslash-in-comment tests; kept in sync with PyBNF's sibling reader (pybnf/petab/_bngl.py). ruff clean, 21 passed. Refs: PEtab-dev/PEtab#436.
BNGL declarations may carry a leading line label (LineLabel = {Digit}, WS |
Name, ":", [WS]): a legacy .net-style numeric index (`1 L0 1`) or a named label
(`CD14: CD14(...)`). The reader took the label as the entity -- the index as a
parameter name, the label as the seed species. Add _strip_line_label() and apply
it in the parameter and seed-species extractors (a valid BNGL identifier starts
with a letter, so a leading digit-run is unambiguously an index; a compartment
prefix carries `@`, so a bare `Name:` is unambiguously a label).
Surfaced by a writeModel-based differential over the bng_parity corpus (895
community models): 4 models disagreed with BNG2.pl's canonical parse (indexed
params/seed, labeled seed); after this fix, 0 -- parameters/observables/
functions/molecule-types/compartments all match BNG2.pl across the corpus.
Kept in sync with PyBNF's sibling reader. ruff clean, 24 passed.
Refs: PEtab-dev/PEtab#436.
Asserts parse_bngl enumerates the same model entities BNG2.pl does, over 21 curated public community BNGL models (RuleHub, BNGL-Models) under tests/v1/bngl_corpus/. BNG2.pl's answers are cached in golden.json -- the entity name sets it emits from `writeModel` (its canonical parse, no network generation) -- so the test needs NO BNG2.pl and runs anywhere; it compares the reader against the frozen oracle. Seed species are compared by molecule composition to absorb BNG2.pl's pattern canonicalization (t vs t(), component reordering, @compartment prefix vs suffix). The models exercise every hardened reader path: line continuations, indexed and labeled declarations, block aliases, the $ clamp, compartmental BNGL, energy patterns, states/bonds, component reordering, bare-molecule seed species. The golden is regenerated deliberately (needs BNG2.pl) via `python tests/v1/test_bngl_corpus.py` and reviewed as a diff. Mirrors PyBNF's live-BNG2.pl gate (lanl/PyBNF); validated there over the full 895-model bng_parity corpus (894/894 BNG2.pl-accepted models agree). ruff clean; 21 passed without BNG2.pl. Refs: PEtab-dev/PEtab#436.
…ies`)
The grammar doc lists `molecules` (for `molecule types`) and `rules` (for
`reaction rules`) as block aliases, but BNG2.pl 2.9.3 -- the reference this
reader targets -- REJECTS both ("Could not process block type 'molecules' /
'rules'"). Honoring them let the reader enumerate entities from a block BNG2.pl
refuses, i.e. accept models the reference rejects. Restrict _BLOCK_ALIASES to
`species` (for `seed species`), which BNG2.pl accepts and in fact emits as its
own canonical seed-species spelling. Verified empirically against BNG2.pl 2.9.3.
The corpus gate is unchanged (no fixture uses the dropped aliases; golden
regenerates identically). ruff clean; 24 passed. Refs: PEtab-dev/PEtab#436.
The 21 third-party .bngl fixtures under tests/v1/bngl_corpus/ are unmodified (or, for Barua_2009, one-line-patched) copies of files already published in RuleWorld/RuleHub and wshlavacek/BNGL-Models. Rather than vendoring ~2600 lines of someone else's model text, fetch the same 21 files on demand from their pinned upstream commits, sha256-verified against the exact bytes reviewed here. golden.json and README.md stay committed -- they're this repo's own oracle/test code, not sourced from anywhere upstream. See scripts/fetch_bngl_corpus_demo.py for the fetcher.
Add tests/v1/fetch_bngl_corpus.py: fetches the 21 .bngl fixtures backing tests/v1/test_bngl_corpus.py from their pinned upstream commits (RuleHub, BNGL-Models) via jsdelivr's GitHub CDN, sha256-verified against the exact bytes reviewed. No git/subprocess use, no execution of fetched content. Wire it into CI as a step before the unit tests run, gitignore the fetched .bngl files (golden.json/README.md stay tracked), and give test_reader_matches_bng2_golden a real skip reason pointing at the fetch script when the corpus hasn't been materialized yet.
dweindl
force-pushed
the
bngl-corpus-fetch-on-demand
branch
from
August 12, 2026 13:08
d1124cf to
170f219
Compare
dweindl
marked this pull request as ready for review
August 13, 2026 10:14
dilpath
approved these changes
Aug 13, 2026
dilpath
left a comment
dilpath
left a comment
Member
There was a problem hiding this comment.
Overall, fine for me since it doesn't affect much pre-existing code.
Will there be example PEtab problems with BNGL models here in the tests, or in the petab_test_suite?
Or some guide for BNGL users to describe how e.g. only species exported via BNGL observables are valid for PEtab observable formulae?
|
|
||
| python tests/v1/test_bngl_corpus.py | ||
|
|
||
| Vendored from public repos (RuleWorld/RuleHub, wshlavacek/BNGL-Models). |
| _RULEHUB, | ||
| "Published/Barua2009/Barua_2009.bngl", | ||
| "26ca5053c4a340b597b2d839edd736469fc14cc2a03cf96b0447b7537089c454", | ||
| ("atoll=>", "atol=>", 1), |
Member
There was a problem hiding this comment.
Should rather be an rulehub PR? Then remove the "repair" feature.
Comment on lines
+25
to
+31
| # Running this file directly makes the interpreter prepend its own directory | ||
| # to sys.path. The `math/` directory then shadows the stdlib `math` module, | ||
| # resulting in an ImportError. Drop that entry before importing anything that | ||
| # could pull in `math`. | ||
| _SCRIPT_DIR = str(Path(__file__).resolve().parent) | ||
| if sys.path and sys.path[0] == _SCRIPT_DIR: | ||
| del sys.path[0] |
Member
There was a problem hiding this comment.
Clean up PATH after its modification is no longer needed, instead of here?
Contributor
|
There are PEtab lessons included in the PyBNF tutorial: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #501 (BNGL model support, by @wshlavacek - full credit for the reader implementation and model corpus selection, all of which this branch carries forward unchanged).
.bnglfiles and downloads them on demand