ci: give the recovery tier a home in integration.yml; restore Boehm to per-push - #518
Merged
Merged
Conversation
…o per-push The `recovery` tier (real parameter-recovery fits through the bngsim backend, over an hour) ran nowhere automatically -- deselected from per-push CI (#515), absent from the pre-push hook, and not part of integration.yml's nightly slow job. Add a second job to integration.yml that installs bngsim and runs `-m recovery`. It is gated to workflow_dispatch only for now (not the nightly cron): the wall-clock is unmeasured on a 2-core runner and these are stochastic fits, so it needs one dispatch to size it and a flake policy settled before it goes on a schedule. Escalating is a one-liner (drop the `if:` for nightly, add `continue-on-error` for non-blocking), documented on the job. Also fix the small live regression #515 introduced: the Boehm `test_roadrunner_reproduces_published_data` is roadrunner-backed (bngsim-free, ~3s) but was recovery-marked at the class level, so it stopped running anywhere automatic. Move the class-level recovery marker down to just the bngsim cross-check; the roadrunner leg rejoins the default per-push tier, restoring the one end-to-end real-backend signal #515 dropped. This also aligns the code with the module docstring's stated rule (RoadRunner legs in the normal tier, the bngsim leg gated -m recovery). Closes #517.
wshlavacek
added a commit
that referenced
this pull request
Jul 24, 2026
The recovery tier includes gradient-path parameter-recovery fits whose log-scaled parameters route the gradient into sampling space through jax (test_gradient_*, and the Rijal-2025 real-world cases in test_real_world_examples.py). Those tests RAISE PybnfError rather than skip when jax is absent, so the recovery job -- added in #518 with setup-pybnf's default jax: 'false' -- failed deterministically on its first dispatch: 2 hard failures (lacud5_ode, five_dl1_ode), 125 passed, in ~11.7 min. Not a stochastic tolerance flake; a missing optional dependency. A complete recovery environment provisions jax, so install it here via the composite action's jax input -- exactly as tests.yml's pytest-jax job already does for the same gradient path. The two failing cases pass locally in ~15s with jax present. Follow-up to #518 / #517.
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
1 participant
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.
Closes #517.
The
recoverytier — ~130 real parameter-recovery fits through the bngsim backend, over an hour — had no automated home: deselected from per-push CI (#515), absent from the pre-push hook (deliberately, a multi-minute hook gets the push reaped), and not part ofintegration.yml's nightlyslowjob (that's the 4 analytical statistical tests,bngsim: 'false'). So the fitting loop's only end-to-end verification against a real engine happened when a maintainer typedpytest -m recoveryby hand.Changes
1.
integration.yml— newrecoveryjob.setup-pybnfwithbngsim: 'true', runningpytest -m recovery -n auto, with its owncache-suffix: pybnf-recoveryso it doesn't race theslowjob's uv-cache save on a dispatch.It is dispatch-only for now (
if: github.event_name == 'workflow_dispatch') — deliberately off the nightly cron. The two open questions from the issue are exactly what a first dispatch answers:-n autobuys less there).Escalating once it's sized is a one-liner, documented on the job: delete the
if:to put it on the 08:00 nightly (theon:block already schedules the workflow), or addcontinue-on-error: trueto make it non-blocking.2.
test_petab_sbml_layer.py— fix the small live regression.TestBoehmRecovery::test_roadrunner_reproduces_published_datais roadrunner-backed (bngsim-free, ~3s) but wasrecovery-marked at the class level, so after #515 it ran nowhere automatic — it was the one recovery test actually running in CI before #515. Moved the class-levelrecoverymarker down onto just the bngsim cross-check:test_roadrunner_reproduces_published_data→ default per-push tier (restores the end-to-end real-backend signal ci: install bngsim from PyPI so the simulation suites run in CI (#514) #515 dropped)test_layer_agrees_across_roadrunner_and_bngsim_on_boehm→ keeps@pytest.mark.recovery @pytest.mark.bngsimThis also aligns the code with the module docstring's own stated rule ("RoadRunner legs run in the normal tier; the bngsim leg is gated
-m recovery"), which the class-level marker had contradicted.Verification (local)
integration.ymlparses as valid YAML.-m recoverypicks up the bngsim one; recovery collection 128 → 127.ruff@0.15.14 checkpasses.