Repository navigation
Conversation
…e retry (lanl#1340) The value RHS's retry on a nonnegative-clamped state (GH lanl#135) leaves alone every slot that may be negative by design: a rate-rule target, a fixed or promoted slot, one declared negative (lanl#706). MODEL1812040005's P is a rate-rule target at an exact equilibrium of 0, read by a*P^z (z fractional): summed in the order its species are written its derivative is exactly 0, and reversed the same sums leave -3.6e-27, where P^z is NaN, and the run failed (CV_REPTD_RHSFUNC_ERR). On that retry a slot the clamp leaves alone is read as 0 where it is negative by less than its own atol, as the sensitivity RHS reads one (GH lanl#392, clamp_state_numerically_zero); one negative by more is left alone and still fails where its rate law is undefined. rhs_zero_atol is filled on both run paths (create_cvode_core and the warm cache). Over the 1,300 BioModels jobs, every model that ran on main runs to the bit-identical final state; one more runs: MODEL9811206584, whose log(s) at s = -1.2e-12 ended main's run at t = 95.8.
|
A heads-up before we review this one, which is ours this week: it conflicts with main since #1354 merged (c0877fa). The conflict is in the loop of
Could you rebase it on main? The two rules need combining in that loop. #1370, which is on current main, conflicts with this branch in the same place. We'll review the three PRs that change this clamp (#1300, #1346 and #1370) together, once all three are on current main. |
Resolve clamp_state_nonneg: keep main's spare (a masked slot at its pole keeps its value) and lanl#1340's read of an unmasked slot negative by less than its atol as 0.
|
Thanks. We reviewed this independently, Opus 5.5 and Fable 5.1, each on our own build of d787eac (a fast-forward on main e425546; main has since moved to edf8876, with no change to For the issue's case the fix does what it says:
But the band is applied whenever the retry runs, to every excluded slot inside it, and that has two effects the tests don't see. One moves a slot that didn't make f non-finite and silently changes the answer (§1). The other turns a fast failure into a run that creeps until the wall-clock timeout (§2). What we checked comes first, so it needn't be redone. What we checked
1. The band moves a slot that didn't make f non-finite
A model built for it, in Antimony: S is drained past 0 and read by
V itself stays at -5e-10; every retry after S < 0 reads it as 0. Each of us built the same fix: a two-stage retry, evaluating at the masked-only clamp first and applying the band only if f is still non-finite. On the interpreted RHS, it passes the nine clamp-rule files (134 passed, 1 skipped, as the baseline) and gives B(10) = 5.0. The same staging would carry to the codegen RHS and the Jacobian retries. It doesn't address §2, where the slot in the band is the cause. 2. A slot that drains past -atol makes the run creep instead of failInside the band the retry reads the slot as 0 and the step succeeds. An ulp outside, f is NaN, CVODE cuts h, and the cut step's prediction lands back inside the band and succeeds. h settles near 1e-16 to 1e-19. Each batch still moves t by a few ulps, which The test file's own shape,
So the outcome depends on where the crossing lands. The band is meant for rounding residue, which never leaves it. A slot genuinely draining past -atol is the "negative by more" case, which should still fail. Here it fails only by
3. The steady-state run test#1370's run test clamps with its own copy, Low
Not checked:
|
Fixes #1340.
What was wrong
MODEL1812040005's species P is a rate-rule target at an exact equilibrium of 0, read by
a_hat = a*P^zwith z fractional. Run in the issue's window, with the whole state compared by name:P^zis NaN and the run fails withCV_REPTD_RHSFUNC_ERRThe right-hand side is the same function in both orders. Summed in another order, its derivative of P at that equilibrium leaves a rounding residue a hair below 0 instead of exactly 0. The original order succeeds by an accident of rounding.
When the RHS goes non-finite, it is retried on a nonnegative-clamped state (GH #135). That clamp leaves alone every slot that may be negative by design (#706): a rate-rule target, such as a membrane potential at −61, a fixed or promoted slot, or one declared negative. So P was left at −3.6e-27, and the retry failed too.
The issue's hunch was right: the clamp mask's indexing (
product_indices, 1-based) is fine. The mask does what it should; the gap is that "negative by design" also caught a value that is numerically zero.The fix (
cvode_simulator.cpp,clamp_state_nonneg)On that retry only, a slot the clamp leaves alone is read as 0 where it is negative by less than its own absolute tolerance. One negative by more is left alone, and still fails where its rate law is undefined. This is the rule the sensitivity RHS already applies (GH #392,
clamp_state_numerically_zero), now on the value RHS too. The tolerance is the run's own per-speciesatol(rhs_zero_atol), filled on both run paths:create_cvode_coreand the warm cache.The retry engages only after the unclamped RHS is non-finite. A model whose RHS stays finite is byte-identical, and the original order of MODEL1812040005 is one.
Verified
MODEL1812040005 in both orders now runs. Each species agrees across the orders to about 1e-12 of its peak, and P to 2e-16 absolute, against an
atolof 1e-9.Corpus A/B. Every one of the 1,300 BioModels ODE jobs, run on main's core and on this one:
MODEL9811206584 is the same mechanism through a different kind of excluded slot: it has no rate rules. On main it fails at t = 95.8 on
log(s)with s = −1.2e-12, inside atol (phi_b_s,phi_r_sNaN). Here s is read as 0 on the retry, sophi_b_sgoes to 0, its limit as s → 0 from above. Its other negative species, r = −2.6e-8, is past atol and is not snapped. Over [0, 90] the new run agrees with RoadRunner to 1.4e-5 (rtol 1e-6), and with itself at 100× tighter tolerances to 2.4e-6. RoadRunner itself fails on this model at t = 91 (CV_TOO_MUCH_WORK), so past that neither engine is a reference.Tests (
python/tests/test_rate_rule_numerically_zero.py)A deterministic model of the mechanism: a rate-rule target
P' = -cfrom P = 0, read byP^0.5in A's decay.create_cvode_core(P^1.5, flat at 0)Mutations, each rebuilt and caught:
Suites:
python/testsandparity_checks/testsgive 17,329 passed, 172 skipped, 1 xfailed. clang-format 17.0.6 and ruff 0.14.0 are clean.