From f2b51d17a1d06b60deb5446811326bd62aea4f87 Mon Sep 17 00:00:00 2001 From: Bill Hlavacek Date: Sun, 23 Aug 2026 16:30:25 -0600 Subject: [PATCH] Raise the bngsim floor to 0.15.0, and make CI install what the project declares PyBNF now requires bngsim 0.15.0. The reason is a contract rather than a feature PyBNF wants. bngsim 0.15.0 publishes event_sensitivities as a real capability key. Until it existed PyBNF answered that question by reading a different key, effective_ic_sensitivity, as a stand-in, which worked only because the two landed a few commits apart. That is a fact about the order of commits, not about what either key means, and it stops being evidence the moment the two are decoupled. Guessing wrong is not symmetric. A build without the event fixes does not refuse the case it cannot handle, it returns a gradient with a term missing, so the fit finishes and reports a number that looks fine. 0.15.0 is the first release where PyBNF can ask the real question. It also carries the codegen cache decline reason PyBNF reads, and a re-vendored RuleMonkey that declines a TotalRate rule where RuleMonkey and NFsim genuinely disagree rather than picking one reading. The floor was written in six places and two had drifted. The CI action pinned bngsim 0.11.35 while pyproject.toml declared 0.12.2. Both were set to the same number on 24 July. pyproject moved twice over the following fortnight and the action never followed, so for three weeks continuous integration tested against a bngsim older than the project said it required and nothing said so. Both files carried a comment asking a person to keep them in sync. A comment was all that held it, so this adds a test. The action does not resolve pyproject.toml, it installs a hand written list that mirrors it, because a leg that must run without bngsim cannot re-resolve. The new test reads both files and fails when they name different version ranges for a package, or when the action never installs something pyproject declares. Both directions fail quietly today. Too low a floor means CI passes on a build the published wheel would reject, and a test dependency the action leaves out does not fail a job either, because the suite that needs it skips instead. A second test covers the same class of problem in the release itself. The version is written by hand into four files. pybnf/__init__.py is the source of truth because pyproject.toml reads it, and CITATION.cff, docs/conf.py and CHANGELOG.md are separate copies a release has to remember to bump. Nothing was checking them. A stale CITATION.cff gives anyone citing PyBNF the wrong version, and a stale docs/conf.py labels the published documentation with the previous release. Both tests were checked by reintroducing each mistake one at a time and watching them fail. The floor also settles a question a third test was leaving open. The warm codegen cache test accepted either a replayed decline reason or silence, because the old pin admitted builds on both sides of the change that writes the reason down beside the cached artifact. 0.15.0 carries it, so the test now asserts the replayed reason word for word. --- .github/actions/setup-pybnf/action.yml | 13 +- pybnf/_bngsim_caps.py | 16 ++- pyproject.toml | 4 +- tests/test_gradient_sens_fallback.py | 16 ++- tests/test_packaging_metadata.py | 177 ++++++++++++++++++++++++- tests/test_sbml_solver_tolerances.py | 8 +- 6 files changed, 209 insertions(+), 25 deletions(-) diff --git a/.github/actions/setup-pybnf/action.yml b/.github/actions/setup-pybnf/action.yml index ee61befea..13e7c5c9b 100644 --- a/.github/actions/setup-pybnf/action.yml +++ b/.github/actions/setup-pybnf/action.yml @@ -113,10 +113,17 @@ runs: # 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. + # sync with pyproject.toml -- and note that asking is no longer the only + # thing holding it there. This pin was set to 0.11.35 on 2026-07-24, when + # that was also pyproject's floor. pyproject then moved to 0.12.0 and to + # 0.12.2, on 2026-08-02 and 2026-08-04, and this line did not follow, so + # for three weeks CI tested against a bngsim older than PyBNF declared it + # needed and nothing said so. tests/test_packaging_metadata.py:: + # test_ci_action_installs_every_dependency_pyproject_declares now reads + # both files and fails when they disagree. BNGSIM_SPEC=() if [ "$BNGSIM" = 'true' ]; then - BNGSIM_SPEC=('bngsim[antimony]>=0.11.35,<1') + BNGSIM_SPEC=('bngsim[antimony]>=0.15.0,<1') fi # The optional pybnf[jax] extra (jax + jaxlib + blackjax) -- the ADR-0059 # HMC reference sampler and the jax-gated gradient-assembly tests. Heavy @@ -145,6 +152,8 @@ runs: # heavy jax stack is instead gated behind the `jax` input above. Keep ALL # of these version bounds in sync with pyproject.toml's [tests] extra -- # anything declared there but missing here silently reverts to a skip. + # tests/test_packaging_metadata.py checks that, for every requirement in + # this file and in pyproject.toml, both name the same version range. uv pip install \ "${BNGSIM_SPEC[@]}" \ "${JAX_SPEC[@]}" \ diff --git a/pybnf/_bngsim_caps.py b/pybnf/_bngsim_caps.py index 1edd74b4a..4db5786b7 100644 --- a/pybnf/_bngsim_caps.py +++ b/pybnf/_bngsim_caps.py @@ -199,11 +199,17 @@ def _empty_capabilities(): # So the flag resolves through three routes, first match wins, and each is # reported by :func:`event_sens_probe` so a refusal can say how it decided: # -# 1. ``features['event_sensitivities']`` -- a dedicated key. bngsim publishes no -# such key today; naming it here costs nothing and means the flag starts -# reading the real answer, in BOTH directions, on the first build that grows -# one. That is #558's ask 1, and it is why this route is checked first even -# though it never fires yet. +# 1. ``features['event_sensitivities']`` -- a dedicated key, and since lanl/bngsim#431 +# a real one. It was named here before it existed, on the reasoning that doing so +# cost nothing and would start the flag reading the real answer, in BOTH directions, +# on the first build that grew one. bngsim 0.15.0 is that build, and the floor is now +# 0.15.0, so this route fires for every install resolved from PyPI. It publishes the +# key on every build, so a ``False`` here is an answer rather than a silence. +# +# Routes 2 and 3 are not dead. A floor binds what a resolver installs, not what a +# build claims, and a bngsim built from source between the version bump and the fixes +# declares a number it does not carry -- which is the whole of why ADR-0119 stopped +# reading version strings. Those routes are what such a build falls through to. # # 2. ``features['effective_ic_sensitivity']`` -- a WITNESS. This key is not the # capability; it reports ``Model.effective_ic_sensitivity``, the dx(0)/dtheta diff --git a/pyproject.toml b/pyproject.toml index 2cc4d6156..1374b5c63 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,7 +32,7 @@ classifiers = [ "Topic :: Scientific/Engineering :: Bio-Informatics", ] dependencies = [ - "bngsim>=0.12.2,<1", + "bngsim>=0.15.0,<1", "dask>=2024.1.0", "distributed>=2024.1.0", "libroadrunner>=1.6.0", @@ -47,7 +47,7 @@ dependencies = [ [project.optional-dependencies] antimony = [ - "bngsim[antimony]>=0.12.2,<1", + "bngsim[antimony]>=0.15.0,<1", ] # The PEtab math translator (pybnf.petab.formula, ADR-0035): an expression # observableFormula is parsed/serialized through petab's sympy-backed math grammar diff --git a/tests/test_gradient_sens_fallback.py b/tests/test_gradient_sens_fallback.py index 156f10a4f..4f7018292 100644 --- a/tests/test_gradient_sens_fallback.py +++ b/tests/test_gradient_sens_fallback.py @@ -481,11 +481,12 @@ def test_a_declined_model_is_reported_identically_cold_and_warm(tmp_path, came back empty. The verdict is the half ADR-0121 pins, and it must not depend on the cache. The - reason is prose, and the pin ``bngsim>=0.12.2,<1`` admits builds on both sides of a - recent upstream change: a build from before it generates no source on a warm cache - and so replays no reason, while a build that carries the note beside the cached - artifact replays the reason a cold build gave, in the same words. Both are fine - here, and a replayed reason is strictly more useful than silence. + reason is prose, and it used to be allowed to go missing: the pin ``bngsim>=0.12.2,<1`` + admitted builds on both sides of the upstream change that writes the reason into a note + beside the cached artifact, so a warm cache could legitimately say nothing. The floor is + now ``bngsim>=0.15.0,<1``, which is the release that carries the note, so silence is no + longer one of the answers and the warm reason is asserted word for word against the cold + one. """ cold = _sbml_model(tmp_path, _DECLINED_LAW, 'declined').analytic_sens_rhs_status() assert cold.analytic is False @@ -502,8 +503,9 @@ def test_a_declined_model_is_reported_identically_cold_and_warm(tmp_path, bngsim_sbml_model._ENGINE_TEMPLATE_WARM_ATTEMPTED.clear() warm = _sbml_model(tmp_path, _DECLINED_LAW, 'declined2').analytic_sens_rhs_status() assert warm.analytic is False, 'the verdict must not depend on the codegen cache' - assert warm.reasons in ([], cold.reasons), ( - 'a warm cache either says nothing or replays the cold reason word for word') + assert warm.reasons == cold.reasons, ( + 'the floor guarantees the note beside the cached artifact, so a warm cache must ' + 'replay the cold reason word for word rather than fall silent') # --- the config surface ------------------------------------------------------- # diff --git a/tests/test_packaging_metadata.py b/tests/test_packaging_metadata.py index 6ac461414..c43b95c2d 100644 --- a/tests/test_packaging_metadata.py +++ b/tests/test_packaging_metadata.py @@ -1,5 +1,6 @@ import fnmatch import os +import re import subprocess import sys import textwrap @@ -7,18 +8,180 @@ from pathlib import Path +REPO_ROOT = Path(__file__).resolve().parents[1] +CI_ACTION = REPO_ROOT / '.github' / 'actions' / 'setup-pybnf' / 'action.yml' + + def test_project_metadata_declares_python_floor_and_bngsim_dependency(): - pyproject_path = Path(__file__).resolve().parents[1] / 'pyproject.toml' + pyproject_path = REPO_ROOT / 'pyproject.toml' metadata = tomllib.loads(pyproject_path.read_text()) project = metadata['project'] assert project['requires-python'] == '>=3.11' - # bngsim >= 0.12.0: parameter_scan/bifurcate carry the equilibration's dx/dθ into the scan - # (lanl/bngsim#81) and resolve an on_point hook's own ∂x(0)/∂θ (lanl/bngsim#111), which make - # a scored PRE-EQUILIBRATED dose-response scan differentiable (#532). (0.11.35 added the - # steady-state SteadyStateResult.output_sensitivities of lanl/bngsim#12 for #478; 0.11.34 the - # native carried-state parameter_scan/bifurcate + named saved states of lanl/bngsim#11 for #474.) - assert 'bngsim>=0.12.2,<1' in project['dependencies'] + # bngsim >= 0.15.0 is bought by a CONTRACT, not by a feature PyBNF wants. lanl/bngsim#431 + # publishes `event_sensitivities` as a real capabilities() feature key. Until it existed + # PyBNF read `effective_ic_sensitivity` as a WITNESS for the same thing (ADR-0119), which + # was sound only because lanl/bngsim#155 happened to land a few commits after the event + # fixes -- a fact about commit ordering that stops being evidence the moment the two are + # decoupled, silently. Guessing wrong here is not symmetric: a build without those fixes + # does not refuse, it returns a finite gradient with a term missing, so the fit converges + # and reports a plausible number. 0.15.0 is the first release where PyBNF asks the real + # question. It also carries the codegen-cache decline reason PyBNF reads (#647), and, on + # the refusal side, a RuleMonkey re-vendor that declines a TotalRate rule where RuleMonkey + # and NFsim genuinely disagree rather than picking a reading. + # (0.12.2/0.12.0 bought the carried-state parameter_scan/bifurcate sensitivities of + # lanl/bngsim#81 and #111 for #532; 0.11.35 the steady-state + # SteadyStateResult.output_sensitivities of lanl/bngsim#12 for #478; 0.11.34 the native + # carried-state parameter_scan + named saved states of lanl/bngsim#11 for #474.) + assert 'bngsim>=0.15.0,<1' in project['dependencies'] + + +def _canonical_name(name): + """PEP 503 normalization, so `pytest-xdist` and `pytest_xdist` are one name.""" + return re.sub(r'[-_.]+', '-', name).lower() + + +_REQUIREMENT = re.compile( + r'^([A-Za-z][A-Za-z0-9._-]*)' # distribution name + r'(?:\[[^\]]*\])?' # extras, which the two files are allowed to differ on + r'\s*(.*)$' # the version specifier, or nothing at all +) + + +def _split_requirement(text): + """``'bngsim[antimony]>=0.15.0,<1'`` -> ``('bngsim', '>=0.15.0,<1')``. + + Extras are dropped on purpose: the CI action installs ``bngsim[antimony]`` where + pyproject's runtime list declares bare ``bngsim``, and that difference is deliberate + (the extra is what unskips the bngsim_antimony-marked tests). The version range is + the part that has to agree. Returns None for a string that is not a requirement. + """ + match = _REQUIREMENT.match(text.strip()) + if match is None: + return None + name, specifier = match.groups() + return _canonical_name(name), specifier.replace(' ', '') + + +def _requirements_quoted_in(text): + """Every single-quoted requirement in a YAML file, keyed by distribution name. + + The CI action is a shell script embedded in YAML, so its requirements are ordinary + single-quoted shell words -- in the `uv pip install` argument list, in the BNGSIM_SPEC + and JAX_SPEC arrays, and in the `petab-spec` input's default. Quotes are matched + within a single line and comment lines are dropped, because the surrounding YAML prose + is full of apostrophes ("pyproject.toml's", "the action's") that otherwise pair up with + each other and swallow the real strings. + """ + found = {} + for line in text.splitlines(): + if line.lstrip().startswith('#'): + continue + for quoted in re.findall(r"'([^'\n]*)'", line): + parsed = _split_requirement(quoted) + if parsed is not None: + found.setdefault(parsed[0], parsed[1]) + return found + + +def test_ci_action_installs_every_dependency_pyproject_declares(): + """The CI action and pyproject.toml must name the same version range for every package. + + The action does not install PyBNF's dependencies by resolving pyproject.toml. It + installs a hand-written list that mirrors it, because a `bngsim: false` leg has to be + able to leave bngsim out and a re-resolve would pull it back in. Two hand-maintained + copies of one list drift, and this one did: the action pinned `bngsim>=0.11.35` while + pyproject declared `bngsim>=0.12.2`. The two were set to the same number on 2026-07-24; + pyproject moved twice over the following fortnight and the action never followed, so for + three weeks continuous integration tested against a bngsim older than the project said + it required. Nothing caught it. Both files carried a comment asking a human to keep them + in sync, which is what a comment can do. + + The failure is quiet in both directions. Too low a floor means CI passes on a build the + published wheel will not accept. A package declared in pyproject's [tests] extra but + absent from the action does not fail a job either -- the suite that needs it + `importorskip`s and reports as skipped. + """ + metadata = tomllib.loads((REPO_ROOT / 'pyproject.toml').read_text()) + declared = {} + for requirement in ( + metadata['project']['dependencies'] + + metadata['project']['optional-dependencies']['tests'] + ): + name, specifier = _split_requirement(requirement) + declared[name] = specifier + + installed = _requirements_quoted_in(CI_ACTION.read_text()) + + missing = sorted(name for name in declared if name not in installed) + assert not missing, ( + 'pyproject.toml declares these but %s never installs them, so the tests that ' + 'need them skip in CI instead of failing: %s' % (CI_ACTION.name, ', '.join(missing)) + ) + + drifted = { + name: (declared[name], installed[name]) + for name in declared + if declared[name] != installed[name] + } + assert not drifted, ( + 'the CI action and pyproject.toml disagree about a version range ' + '(name: pyproject wants, action installs): %s' % drifted + ) + + +def test_the_version_is_the_same_everywhere_it_is_written_down(): + """One release version, written by hand into four files, must agree in all of them. + + pyproject.toml does not carry the number: it declares `dynamic = ["version"]` and reads + `pybnf.__version__`, which makes `pybnf/__init__.py` the source of truth and the built + wheel correct by construction. The other three are separate copies that a release has to + remember to bump, and nothing has been checking them. A stale `CITATION.cff` tells anyone + citing PyBNF the wrong version, and a stale `docs/conf.py` labels the published + documentation with the previous release. + + The changelog is checked in the same breath because it is the same ritual and the same + mistake. `[Unreleased]` is promoted to a dated heading at release time, so the newest + versioned heading is the released version at every commit -- both before a release, when + the promotion has not happened and neither has the bump, and after it, when both have. + Bumping one without the other is what this catches. + """ + init_text = (REPO_ROOT / 'pybnf' / '__init__.py').read_text() + match = re.search(r'^__version__ = [\'"]([^\'"]+)[\'"]', init_text, re.MULTILINE) + assert match is not None, 'pybnf/__init__.py does not define __version__' + version = match.group(1) + + # The mechanism that makes __init__.py authoritative. If this ever changes, the rest of + # this test is checking copies against something that is no longer the original. + metadata = tomllib.loads((REPO_ROOT / 'pyproject.toml').read_text()) + assert 'version' in metadata['project']['dynamic'] + assert metadata['tool']['setuptools']['dynamic']['version'] == { + 'attr': 'pybnf.__version__' + } + + citation = (REPO_ROOT / 'CITATION.cff').read_text() + assert re.search(r'^version: %s$' % re.escape(version), citation, re.MULTILINE), ( + "CITATION.cff does not say version %s -- anyone citing PyBNF gets the wrong " + 'release' % version + ) + + conf = (REPO_ROOT / 'docs' / 'conf.py').read_text() + assert re.search(r"^version = '%s'$" % re.escape(version), conf, re.MULTILINE), ( + "docs/conf.py's `version` is not %s" % version + ) + # `release` carries a leading v; `version` does not. Both are bumped by hand. + assert re.search(r"^release = 'v%s'$" % re.escape(version), conf, re.MULTILINE), ( + "docs/conf.py's `release` is not v%s" % version + ) + + changelog = (REPO_ROOT / 'CHANGELOG.md').read_text() + headings = re.findall(r'^## \[v?([^\]]+)\]', changelog, re.MULTILINE) + released = [h for h in headings if h != 'Unreleased'] + assert released, 'CHANGELOG.md has no released version heading' + assert released[0] == version, ( + "CHANGELOG.md's newest released heading is %s but __version__ is %s -- one of the " + 'two was bumped without the other' % (released[0], version) + ) def test_every_pybnf_subpackage_is_shipped(): diff --git a/tests/test_sbml_solver_tolerances.py b/tests/test_sbml_solver_tolerances.py index 8f73410ce..935dbc4aa 100644 --- a/tests/test_sbml_solver_tolerances.py +++ b/tests/test_sbml_solver_tolerances.py @@ -925,8 +925,12 @@ def test_the_backend_default_atol_is_what_corrupted_those_sensitivities(tmp_path simply less catastrophic than it was. So each threshold now sits a decade under the *worse* of the two measurements -- the - rule the two-scale test above states -- and both hold across the declared - ``bngsim>=0.12.2,<1`` range rather than on the one release that drew them. + rule the two-scale test above states -- rather than on the one release that drew them. + The declared range is now ``bngsim>=0.15.0,<1``, which puts both of those measurements + below the floor. That leaves the guard conservative rather than stale: raising the floor + removed the worse end of the pair, and a threshold set a decade under the worse end is + still under the better one. It has not been re-derived on 0.15.0, and it does not need + to be until a release moves the numbers back up. """ model = _piecewise_model(tmp_path, atol=_BNGSIM_DEFAULT_ATOL) model.enable_output_sensitivities(params=['k0', 'k1', 'k2'])