test(rr_parity): BIOMD0000000627's divergence is RoadRunner stepping over an ungated ramp (#482) - #483
Merged
Conversation
…over an ungated ramp (#482) Reported as a 0.13.0 regression: on BIOMD0000000627 the stimulus that switches on at t = 200 appeared to take effect ~1 s early, smeared over the 3 s before it, against a premise that the pre-switch trajectory is a steady state and so every pre-200 sample must sit on a flat baseline. It is not a steady state, and they must not. Jolivet2015 gates its stimulus on an assignment rule -- `is_stimulated = piecewise(0, (time<=200) or (time>=t_0+t_end), 1)` -- but the blood-flow multiplier beside it is UNGATED. `f_CBF_dyn` is a logistic centred at `t_0 + t_1 - 3` = 199, so perfusion (`F_in = F_0*f_CBF_dyn`) runs 1.0000004 -> 1.4157864 over t = 196..200: the flow response leads the stimulus by one second, by construction. species_24 (CO2) is washed out along that ramp, so the correct trajectory is already falling well before the switch. The reported `is_stimulated` / `v_stim` / `vSTIM` traces reading 0 through t = 200 are correct and beside the point -- the pre-200 motion is not in them. bngsim resolves the ramp because the #305 crossing stop lands the step on t = 200, which pulls the step size down on the approach. The ramp itself is smooth: it brackets no #72 root, and the model carries no floor/ceiling/modulo, so `_periodic_disc_max_step` is None and #274 -- the other suspect, and the only one that could have changed a step bound -- never enters. Remove the #305 stop and bngsim reproduces RoadRunner's answer bit for bit, which is what bngsim <= 0.12.2 did. The regression window is right; the sign is not. RoadRunner at the sweep grid steps clean over the ramp and reports the flat baseline 2.2084924 through t = 200 (worst species_24 at t = 199.6: bngsim 1.9434505 vs RR 2.2084924, reld 0.136). Its answer gets WORSE as tol tightens: at rtol/atol 1e-11/1e-14 and 1e-12/1e-16 it stays flat through t = 200.4 too, then missing part of the stimulus response as well. Tol-stable wrongness is the signature of a feature never sampled, and a denser output grid cannot help because CV_NORMAL interpolates output points -- so TOL_OVERRIDES is not the remedy. OBSERVED behaviour only; RR source not inspected, no mechanism asserted. Third-oracle attribution. Three independent converged references agree with bngsim to 7 digits at t = 197.6/198.8/199.6/200.0/200.4 (2.2081597/2.1457629/1.9434505/1.9173758/1.9130018): * RoadRunner with maximum_time_step = 0.05; * RoadRunner with maximum_time_step = 0.01, agreeing with the first to 7 digits and so converged; * RoadRunner with NO step bound at all, simply restarted at t = 190 so its own step control has to resolve the window -- the reference that rules out the bound itself being the artifact. Suite-side only; no bngsim behaviour changes. build_ode_jobs.py cannot run in a checkout without $BIOMODELS_SEDML_DIR (and would churn _meta.generated), so ode_jobs.json is re-baked for the one record through the same Override.to_dict() + json separators write_manifest uses, verified to leave the other 1322 records and _meta byte-identical. parity_checks/tests: 479 passed, 17 skipped.
wshlavacek
added a commit
that referenced
this pull request
Aug 25, 2026
…the reference, not on bngsim (#485) (#486) `INVALID_REFERENCE["MODEL2002070001:ode"]` went stale, and the 0.15.0 sweep said so: `STALE invalid-reference entry: natural=DIFF, rr_finite=False`. Half the premise died in 0.14.0 -- the #353 unit-volume substitution for a non-finite compartment size let bngsim integrate the model -- so the natural verdict resurfaced as a DIFF with max_rel_err=inf, 606/707 cells failing. The other half is untouched: RoadRunner still returns NaN. The entry wants relaxing, not pruning. The model is well posed and the NaN is not in its dynamics. Both compartments declare size="NaN", but every species is hasOnlySubstanceUnits="true", the dynamics are 7 rate rules with 0 reactions, and no <math> block in the file names mucosa or lumen. So nothing reads a size and the IVP is well posed in amounts. RoadRunner carries species as concentrations, so x1..x6 -- the six species in those compartments -- are NaN from t=0, while x7, the one rate-rule variable that is a parameter rather than a species, stays finite. That is the control: the NaN reaches RR's state through the compartment, not through the dynamics. At the sweep tolerance (rtol 1e-9 / atol 1e-12, t=0..100, 101 points), re-verified here against bngsim 0.15.0 / RoadRunner 2.9.2 / AMICI 1.0.1: * bngsim 707/707 cells finite; * RoadRunner runs without raising, 101/707 finite -- the finite column is x7, and it agrees with bngsim cell for cell; * AMICI confirms bngsim's trajectory independently: 0/707 cells failing, max_rel_err 0.0. `_apply_invalid_reference` now keys the premise on the reference alone (RR ran and emitted non-finite output) rather than also requiring bngsim to have failed. Two deliberate departures from the issue's `rr_finite is False or rr_finite is None`: `None` is not admitted, because a segfaulted child records the same absence as an RR raise and the dead-worker guard exists for that; and a natural PASS is not admitted, because a reference non-finite only in a column the comparison never used was usable for the comparison. What the dropped "bngsim failed too" clause was really doing was guarding against burying a bngsim defect, so it is replaced rather than dropped. `_reference_nonfinite_covers` re-runs the same `_core.differ` verdict over just the columns the reference kept finite, and the override holds on a DIFF only if that passes -- a real bngsim divergence beside the NaN leaves a failing cell in a finite reference column and the row stays a DIFF. Restricting the columns can only tighten the re-run (`scale` is the peak over the retained block), so the claim it licenses is never a loosening. BAD_TEST rather than PASS: PASS is the #482/#483 treatment for a divergence attributed to the reference engine, and here it would score RoadRunner as having validated a model it answered NaN on. Both buckets are non-scoring either way. Suite-side only; no bngsim behaviour changes. build_ode_jobs.py cannot run in a checkout without $BIOMODELS_SEDML_DIR (and would churn _meta.generated), so ode_jobs.json is re-baked for the one record through the same Override.to_dict() + json separators write_manifest uses, verified to leave the other 1322 records and _meta byte-identical. The amici_parity README's contrast with this override is updated: it turned on the clause this commit removes. Full ODE sweep on this branch: 1237 PASS / 27 REFERENCE_FAILED / 54 BAD_TEST / 5 TIMEOUT over 1323 jobs -- no DIFF, no EXCEPTION, and no override flagged stale. parity_checks/tests: 494 passed, 17 skipped.
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 #482.
#482 reported that on BIOMD0000000627 the stimulus gated at
t = 200takeseffect ~1 s early, smeared over the 3 s before it, and bisected the change to
0.12.2 → 0.13.0. The window is right. The sign is not: 0.13.0 fixed this model,
and the "baseline" the report measures against is an under-resolved RoadRunner
answer that bngsim ≤ 0.12.2 happened to match.
The premise
It is not a steady state. The stimulus is gated:
The blood flow beside it is not:
F_in = F_0 * f_CBF_dyn, and that multiplier runs:f_CBF_dynPerfusion is up 42% by
t = 200, having risen almost entirely in the secondbefore it — the flow response leads the stimulus by one second, by construction.
CO2 (
species_24) is washed out by flow, so the correct trajectory is alreadyfalling well before the switch.
The report's observation that
is_stimulated,v_stimandvSTIMall read 0through
t = 200is correct and beside the point: the pre-200 motion is not inthem.
Evidence
Three independent converged references, all agreeing with bngsim to 7 digits:
f_CBF_dynThe third reference is the one that rules out the step bound itself being the
artifact: no bound at all, just RoadRunner restarted at
t = 190so its own stepcontrol has to resolve the window. It lands on bngsim's numbers exactly.
RoadRunner's default is unconverged and gets worse as tolerance tightens — at
rtol/atol 1e-11/1e-14 and 1e-12/1e-16 it reports the flat 2.2084924 through
t = 200.4as well, now missing part of the stimulus response too. Tol-stablewrongness is the signature of a feature never sampled; a denser output grid
cannot help, because
CV_NORMALinterpolates output points. SoTOL_OVERRIDESis not the remedy here, and
KNOWN_ARTIFACTis.Why 0.13.0 changed it
7b9140b(#305). The crossing stop lands the step ont = 200, which pulls thestep size down on the approach and incidentally resolves the ramp. The ramp
itself is smooth — it brackets no #72 root and earns no bound of its own.
Remove the #305 stop and bngsim reproduces RoadRunner's default answer bit for
bit (last column above) — which is exactly what bngsim ≤ 0.12.2 did.
The other suspect,
0c83642(#274), cannot be involved: the model contains nofloor/ceiling/modulo, so_periodic_time_disc_max_stepreturnsNonebefore it ever reaches the #274 gate (
model._periodic_disc_max_stepisNoneat HEAD, and was on both sides of the window).
Two of #482's own "what it is not" bullets read the other way once this is clear:
max_step=0.4changing nothing is confirmation that 0.15.0 is already converged,and the intact switch traces are measuring the wrong variable.
What this changes
Suite-side only; no bngsim behaviour changes.
parity_checks/rr_parity/overrides.py— aKNOWN_ARTIFACTentry forBIOMD0000000627:ode, citing the evidence above.parity_checks/rr_parity/ode_jobs.json— the one job record re-baked.build_ode_jobs.pycannot run in a checkout without$BIOMODELS_SEDML_DIR(and would churn
_meta.generated), so the record is regenerated through thesame
Override.to_dict()+ json separatorswrite_manifestuses, verified toleave the other 1322 records and
_metabyte-identical.CHANGELOG.md— under Unreleased.The application note's coverage table keeps this model as validated; it is the
oracle that needed the note, not bngsim.
parity_checks/tests: 479 passed, 17 skipped.