From 97106273b35e58739c059a03e185946944f5f702 Mon Sep 17 00:00:00 2001 From: Bill Hlavacek Date: Fri, 24 Jul 2026 12:20:31 -0600 Subject: [PATCH] ci: install bngsim from PyPI so the simulation suites run in CI (#514) tests.yml ran the whole suite under PYBNF_NO_BNGSIM=1 on the premise that bngsim was a private, macOS-only wheel. It is neither: bngsim 0.11.35 -- exactly the version pyproject.toml pins -- is on public PyPI with manylinux_2_28_x86_64 wheels for cp310-cp313. The premise cost 323 bngsim/nfsim/rulemonkey/sbml-marked tests, including 14 dedicated tests/test_bngsim_*.py files, none of which had ever run on a hosted runner; their only gate was one maintainer's pre-push hook. The published wheel is fully capable -- NFsim, RuleMonkey, libsbml, codegen, forward and steady-state output sensitivities all compile in. Only antimony_import needs a companion package (antimony, also a manylinux wheel), so the composite action installs bngsim[antimony] and the bngsim_antimony tests unskip too. - setup-pybnf grows a `bngsim` input (default true). A leg that must stay bngsim-less passes 'false' and keeps PYBNF_NO_BNGSIM=1: py3.14 (no cp314 wheel, and it now doubles as the guard on the bngsim-free code paths), docs (autodoc never simulates), the native-BNGL petab leg, and integration.yml's analytical slow tier. - The pytest matrix moves to `include` so 3.14 can carry no-bngsim: '1'. On the other legs PYBNF_NO_BNGSIM is empty, which _bngsim_caps reads as unset. - The marker expression becomes `-m "not slow and not recovery"`. Naming -m REPLACES the addopts default, so the old command was also selecting the recovery tier -- free only because 129 of its 130 tests skip without bngsim. With bngsim present they are real parameter-recovery fits and the job would run over an hour. - test_fd_acceptance_gate_estimated_scale is bngsim-marked and fits a loguniform parameter, whose log-space chain rule autodiffs through jax -- an extra CI does not install. Its twelve siblings declare pytest.importorskip('jax'); this one did not, so it failed rather than skipped the moment bngsim arrived. Declared. Verified by mirroring the CI install exactly (PyPI bngsim, not the dev wheel) and running the job's command: 3221 passed, 78 skipped, 0 failed. No bngsim-family test skips remain; the residual skips are the jax/arviz/ matplotlib extras CI has never installed. Co-Authored-By: Claude Opus 4.8 --- .github/actions/setup-pybnf/action.yml | 74 ++++++++++++++++++-------- .github/workflows/docs.yml | 14 +++-- .github/workflows/integration.yml | 12 +++-- .github/workflows/tests.yml | 52 +++++++++++++----- .pre-commit-config.yaml | 33 +++++++----- tests/README_integration.md | 7 ++- tests/test_gradient_assembly.py | 3 ++ 7 files changed, 136 insertions(+), 59 deletions(-) diff --git a/.github/actions/setup-pybnf/action.yml b/.github/actions/setup-pybnf/action.yml index 4d0f74bde..8632ffa19 100644 --- a/.github/actions/setup-pybnf/action.yml +++ b/.github/actions/setup-pybnf/action.yml @@ -1,16 +1,29 @@ -name: Set up pybnf (bngsim-less) +name: Set up pybnf description: >- - Install BioNetGen (BNG2.pl) plus pybnf and its public runtime/test - dependencies into a uv venv, deliberately omitting the private, macOS-only - bngsim wheel. Tests run under PYBNF_NO_BNGSIM=1 so bngsim-marked tests skip - (see tests/conftest.py). BNG2.pl is still required: Configuration validation - execs `BNG2.pl -v` for any BNGLModel regardless of bngsim, and the analytical - tests aside, most of the suite builds such configs. + Install BioNetGen (BNG2.pl) plus pybnf and its runtime/test dependencies into + a uv venv. bngsim comes from public PyPI by default (`bngsim` input), so the + bngsim-marked suites actually run on hosted runners; pass `bngsim: 'false'` + for a leg that must stay bngsim-less (a python with no cp wheel, or a job + that does not simulate at all), and run it under PYBNF_NO_BNGSIM=1 so the + bngsim-marked tests skip cleanly (see tests/conftest.py). BNG2.pl is required + either way: Configuration validation execs `BNG2.pl -v` for any BNGLModel + regardless of bngsim, and the analytical tests aside, most of the suite + builds such configs. inputs: python-version: description: Python version for the venv (e.g. "3.11", "3.12"). required: true + bngsim: + description: >- + Install bngsim (from public PyPI) alongside the rest of the deps. + Default "true". Set "false" on a leg where bngsim must be absent: + bngsim publishes manylinux_2_28_x86_64 wheels for cp310-cp313 only, so a + newer interpreter would fall back to building the sdist (C++17 toolchain, + CMake, Ninja) -- and jobs that never simulate (docs) do not need it. A + "false" leg must also set PYBNF_NO_BNGSIM=1 when it runs pytest. + required: false + default: 'true' petab-spec: description: >- The petab requirement to install. Defaults to stock PyPI petab. A CI leg @@ -70,24 +83,39 @@ runs: tar -xzf "$RUNNER_TEMP/bionetgen.tar.gz" -C "$RUNNER_TEMP" echo "BNGPATH=$RUNNER_TEMP/BioNetGen-2.9.3" >> "$GITHUB_ENV" - - name: Create venv and install pybnf without bngsim + - name: Create venv and install pybnf shell: bash + env: + BNGSIM: ${{ inputs.bngsim }} + PETAB_SPEC: ${{ inputs.petab-spec }} run: | # Build the venv from the setup-python interpreter (see note above), not # a uv-managed one. uv venv --python "${{ steps.python.outputs.python-path }}" - # Mirror pyproject.toml's runtime deps MINUS bngsim (private, mac-only - # wheel, not on PyPI), plus the test stack (pytest, hypothesis, - # pytest-xdist) and petab. petab is BOTH the PEtab v2 export/BnglModel - # oracle (ADR-0025/0026 -- BNG2.pl is installed above, so model-level - # validation runs here too) AND the runtime math translator backing the - # expression observableFormula path (pybnf[petab], ADR-0035, petab.v2.math); - # stock PyPI petab carries the math grammar, so one install covers both. - # The exact petab requirement is the `petab-spec` input (default stock - # PyPI; a CI leg overrides it with the Step B fork to run the NATIVE - # BNGL loader). Keep these version bounds in sync with pyproject.toml; - # bngsim is the only intentional omission. + # bngsim is on public PyPI (manylinux_2_28_x86_64 + macOS + win wheels, + # cp310-cp313), so hosted runners get the real simulation backend -- + # NFsim, RuleMonkey, libsbml, codegen and output sensitivities are all + # compiled into the published wheel. The [antimony] extra is the one + # capability that needs a companion PyPI package (antimony>=3.1.1, pure + # python wheel), and it is what unskips the bngsim_antimony-marked + # tests, so install the extra rather than bare bngsim. Keep the pin in + # sync with pyproject.toml. + BNGSIM_SPEC=() + if [ "$BNGSIM" = 'true' ]; then + BNGSIM_SPEC=('bngsim[antimony]>=0.11.35,<1') + fi + # Mirror pyproject.toml's runtime deps, plus the test stack (pytest, + # hypothesis, pytest-xdist) and petab. petab is BOTH the PEtab v2 + # export/BnglModel oracle (ADR-0025/0026 -- BNG2.pl is installed above, + # so model-level validation runs here too) AND the runtime math + # translator backing the expression observableFormula path + # (pybnf[petab], ADR-0035, petab.v2.math); stock PyPI petab carries the + # math grammar, so one install covers both. The exact petab requirement + # is the `petab-spec` input (default stock PyPI; a CI leg overrides it + # with the Step B fork to run the NATIVE BNGL loader). Keep these + # version bounds in sync with pyproject.toml. uv pip install \ + "${BNGSIM_SPEC[@]}" \ 'dask>=2024.1.0' \ 'distributed>=2024.1.0' \ 'libroadrunner>=1.6.0' \ @@ -101,7 +129,9 @@ runs: 'pytest>=7,<10' \ 'hypothesis>=6,<7' \ 'pytest-xdist' \ - '${{ inputs.petab-spec }}' - # Install pybnf itself without re-resolving deps (which would pull - # bngsim and fail). + "$PETAB_SPEC" + # Install pybnf itself without re-resolving deps: on a bngsim: 'false' + # leg a re-resolve would pull bngsim in through pyproject.toml and + # defeat the point, and everywhere it would re-derive a dependency set + # this step just pinned. uv pip install --no-deps . diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 2fe4b1492..c6a85170d 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -49,14 +49,18 @@ jobs: steps: - uses: actions/checkout@v6 - # Reuse the tests' composite action: it installs pybnf + its PUBLIC runtime - # deps into a uv venv WITHOUT the private, mac-only bngsim wheel -- exactly - # the environment autodoc needs to import pybnf on Linux. (It also fetches - # BNG2.pl, unused here but harmless; the win is one source of truth for the - # dependency list.) + # Reuse the tests' composite action: it installs pybnf + its runtime deps + # into a uv venv -- exactly the environment autodoc needs to import pybnf + # on Linux. (It also fetches BNG2.pl, unused here but harmless; the win is + # one source of truth for the dependency list.) - uses: ./.github/actions/setup-pybnf with: python-version: '3.12' + # autodoc only imports pybnf; it never simulates. Skip the bngsim + # install and build under PYBNF_NO_BNGSIM=1 (below) so the docs job + # stays lean -- and so a docs build keeps proving that pybnf imports + # cleanly with no backend present. + bngsim: 'false' # Distinct uv cache namespace so this job does not race the tests / # lint jobs' content-addressed cache saves on the same push. cache-suffix: docs diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 1d36cc70d..482ed730e 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -3,10 +3,10 @@ name: integration # Manual only. The slow tier is the 4 statistical-recovery tests against # analytical targets (AnalyticalModel) -- bngsim-free and BNG2.pl-free, ~8 min. # -# NOTE: the bngsim *simulation* suites (true NFsim/RuleMonkey/SBML runs) are -# deliberately NOT in hosted CI -- they need the private, macOS-only bngsim -# wheel. They remain covered by the local pre-push hook (.pre-commit-config.yaml) -# on a dev machine until bngsim ships Linux wheels. +# NOTE: the fast bngsim *simulation* suites (true NFsim/RuleMonkey/SBML runs) +# DO run in hosted CI -- tests.yml installs bngsim from public PyPI (#514). Not +# covered anywhere hosted yet: the `recovery` tier (real parameter-recovery fits +# through the backend, over an hour), which still runs on a dev machine. # # Runs nightly and on manual dispatch. on: @@ -27,6 +27,10 @@ jobs: - uses: ./.github/actions/setup-pybnf with: python-version: '3.12' + # The slow tier is analytical (AnalyticalModel): no simulation + # backend, so no bngsim install, and PYBNF_NO_BNGSIM=1 below keeps it + # honest about that. + bngsim: 'false' - name: Run slow analytical recovery tests env: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 334092c83..512f501c9 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,26 +14,49 @@ jobs: # Every supported version (requires-python >= 3.11; classifiers list # through 3.14). CI is free on this public repo, so we test each claimed # version rather than just the endpoints. - python-version: ['3.11', '3.12', '3.13', '3.14'] - name: pytest (py${{ matrix.python-version }}) + include: + - python-version: '3.11' + - python-version: '3.12' + - python-version: '3.13' + # bngsim publishes cp310-cp313 wheels, so 3.14 would fall back to + # building the sdist (C++17 toolchain, CMake >= 3.20, Ninja) inside + # the pytest job. Keep this leg deliberately bngsim-less instead: it + # is the guard that the bngsim-free code paths still work (a bare + # `pip install pybnf` on a machine with no wheel), which is worth a + # leg of its own regardless. Drop `no-bngsim` here the day a cp314 + # wheel ships. + - python-version: '3.14' + no-bngsim: '1' + name: pytest (py${{ matrix.python-version }}${{ matrix.no-bngsim == '1' && ', bngsim-less' || '' }}) steps: - uses: actions/checkout@v6 - uses: ./.github/actions/setup-pybnf with: python-version: ${{ matrix.python-version }} + bngsim: ${{ matrix.no-bngsim == '1' && 'false' || 'true' }} - - name: Run unit tests (bngsim-less, parallel) + - name: Run unit tests (parallel) env: - # bngsim is a private mac-only wheel; with it absent, conftest.py - # auto-skips bngsim/nfsim/sbml/etc.-marked tests. The remaining suite - # runs against the bngsim-less code paths (and BNG2.pl from the - # composite action). -n auto parallelizes across the runner's cores. - PYBNF_NO_BNGSIM: '1' + # Empty except on the bngsim-less leg, where it is '1'. An empty value + # reads as unset (pybnf/_bngsim_caps.py tests truthiness), so bngsim + # is imported and the bngsim/nfsim/rulemonkey/sbml/antimony-marked + # tests run for real; with '1' conftest.py auto-skips them. + PYBNF_NO_BNGSIM: ${{ matrix.no-bngsim }} + # -m: the default addopts expression is `not slow and not recovery`, and + # naming -m here REPLACES it -- so both tiers must be restated. Omitting + # `not recovery` used to be harmless because 129 of the 130 recovery + # tests skip without bngsim; with bngsim installed they are real + # parameter-recovery fits through the backend and the job would run for + # over an hour. (The cost of deselecting: the 130th, a roadrunner-backed + # Boehm oracle in test_petab_sbml_layer.py, is bngsim-free and no longer + # runs here.) -n auto parallelizes across the runner's cores. + # # --no-sync: do NOT let `uv run` re-resolve the project env from - # pyproject.toml (that would pull the private bngsim and fail). The - # composite action already populated .venv. - run: uv run --no-sync pytest -m "not slow" -n auto + # pyproject.toml -- the composite action already populated .venv with an + # exact dependency set (and on the bngsim-less leg a re-resolve would + # pull bngsim right back in). + run: uv run --no-sync pytest -m "not slow and not recovery" -n auto # The PEtab v2 export oracle (ADR-0025/0026) validates emitted `language: bngl` # problems at MODEL level. On the pytest job above (stock PyPI petab) that runs @@ -51,6 +74,11 @@ jobs: - uses: ./.github/actions/setup-pybnf with: python-version: '3.12' + # This leg is about petab's loader, not simulation: the two oracle + # files it runs are backend-free, so skip the bngsim install (and run + # under PYBNF_NO_BNGSIM=1 below) rather than pay for a wheel nothing + # here exercises. The matrix legs above cover bngsim. + bngsim: 'false' petab-spec: 'petab @ git+https://github.com/wshlavacek/libpetab-python.git@bngl_model_support' # This job also runs 3.12 through setup-pybnf, so without its own # suffix it shares the matrix py3.12 job's cache key and one save loses @@ -71,4 +99,4 @@ jobs: # `language: bngl` through petab's own native BnglModel here (the # register_bngl() monkeypatch collapses to a no-op). This is where the # TestRegisterBngl native-no-op test actually runs instead of skipping. - run: uv run --no-sync pytest -m "not slow" tests/test_petab_export.py tests/test_petab_import.py + run: uv run --no-sync pytest -m "not slow and not recovery" tests/test_petab_export.py tests/test_petab_import.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b2b052a1a..90cefe71d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -8,18 +8,23 @@ # `ruff@0.15.14 check .`). Runs FIRST and takes seconds, so a lint failure # aborts the push immediately instead of after the pytest gate (the pytest # hook does NOT run ruff -- that gap let an F401 reach CI once). -# 2. pytest-bngsim -- the bngsim (mac-only) tests CI *cannot* run, since the -# private bngsim wheel is absent on hosted runners. This is the gate's -# unique value: every other test already runs in CI. Run with `-n auto` -# so the whole gate finishes in well under a minute. +# 2. pytest-bngsim -- the simulation-backend tests, as a fast local smoke. +# These are no longer CI-exclusive: bngsim is on public PyPI and tests.yml +# installs it, so hosted runners now run these files too (#514). The hook +# stays because it is the shortest path from "I broke the backend seam" to +# knowing it -- seconds locally instead of a CI round-trip. Run with +# `-n auto` so the whole gate finishes in well under a minute. # -# Deliberately NOT here: the bngsim-less suite (`PYBNF_NO_BNGSIM=1 pytest -# -m "not slow" -n auto`). CI's tests.yml runs that exact command on every push, -# so mirroring it locally added minutes to the gate for zero extra coverage -- -# and a multi-minute pre-push hook holds the push's SSH connection idle long -# enough for GitHub to reap it ("Connection to github.com closed by remote -# host"), so the push silently never lands. Keeping the gate fast is what makes -# `git push` actually complete. Bngsim-less regressions still surface in CI. +# Deliberately NOT here: the rest of the suite (`pytest -m "not slow and not +# recovery" -n auto`, ~3300 tests). CI's tests.yml runs that on every push, so +# mirroring it locally added minutes to the gate -- and a multi-minute pre-push +# hook holds the push's SSH connection idle long enough for GitHub to reap it +# ("Connection to github.com closed by remote host"), so the push silently never +# lands. Keeping the gate fast is what makes `git push` actually complete. +# +# Also NOT here: the `recovery` tier (real parameter-recovery fits through the +# backend, over an hour). That one runs nowhere automatically -- run it by hand +# when a change touches the fitting loop. # # `uv run --no-sync` uses the project's interpreter without touching the # environment -- the .venv is already populated and uv.lock isn't fully @@ -40,9 +45,9 @@ repos: stages: [pre-push] - id: pytest-bngsim name: pytest (bngsim test suite) - # Every bngsim-area test file: bngsim is mac-only and absent in CI, so - # these run nowhere else. `-n auto` parallelizes across cores to keep the - # gate fast. Add files here as bngsim coverage expands. + # Every bngsim-area test file, as a pre-push smoke (CI runs them too -- + # this is the fast local copy). `-n auto` parallelizes across cores to + # keep the gate fast. Add files here as bngsim coverage expands. entry: >- uv run --no-sync pytest -m "not bionetgen" -n auto tests/test_bngsim_bridge.py diff --git a/tests/README_integration.md b/tests/README_integration.md index e79bd2744..d9b81f2f9 100644 --- a/tests/README_integration.md +++ b/tests/README_integration.md @@ -60,8 +60,11 @@ pytest -m recovery -k m03 # just one model Requirements / behaviour: -- **bngsim** must be installed (the macOS-only C++ wheel) — auto-skips via the - `bngsim` marker otherwise, so this never runs in hosted CI. +- **bngsim** must be installed (public PyPI: `pip install bngsim`) — auto-skips + via the `bngsim` marker otherwise. Hosted CI installs bngsim and runs the + bngsim-marked suites, but explicitly deselects this tier + (`-m "not slow and not recovery"`): the fits across all 16 recovery-marked + files run well over an hour. It stays a dev-machine tier. - **BNG2.pl** must be resolvable (via `BNGPATH`): bngsim is a simulation engine, not a network generator, so rules→`.net` expansion runs once per fit at setup. `recovery_harness.require_bng2pl` skips the tier if it isn't found. diff --git a/tests/test_gradient_assembly.py b/tests/test_gradient_assembly.py index 857d133cf..2d1b4bd9b 100644 --- a/tests/test_gradient_assembly.py +++ b/tests/test_gradient_assembly.py @@ -2980,6 +2980,9 @@ def test_fd_acceptance_gate_estimated_scale(model_kind): chain rule end-to-end on the real bngsim decay net. The scale never enters the data fit, so its whole gradient comes from the constraint's scalar column -- the estimated-noise pattern, applied to a qualitative constraint.""" + # ``s_q`` is loguniform, so the scale column's log-space chain rule autodiffs through jax + # (the optional pybnf[jax] extra) -- same declaration as every other log-scale leg here. + pytest.importorskip('jax') obj = ChiSquareObjective() sigma = 5.0 model_name = 'e2e_ode_decay'