Repository navigation
cvode: read the restart test's band from the error weights, in quadrature (#907) - #1329
Merged
wshlavacek merged 3 commits intoOct 29, 2026
Merged
Conversation
…ture (lanl#907) Cases 5 and 3 of lanl#907. A plain run restarts at a state-switch root only where the solver's trajectory goes past the surface by more than the band the tolerance leaves the residual, times 10·√N. The band read each species' atol from the run's static tolerance and summed the species one by one: - under TrackingAtol(decades=6, ceiling=1e-8), CVODE holds S ~ 1e-6 to 2e-14, but the band read the 1e-8 ceiling, and a window 5e-9 wide in S, closed by a clock, was lost on three phases of five; - over an observable summing 1,000 species of 1e-3, the band carried 1,000 atols, 3.2e-3 with the factor, past the window's whole excursion of 1e-3, and it was lost on three phases of five. The band's unit for each species is now what CVODE's own error-weight rule holds it to at the state read (tolerance_unit: rtol·|x| + atol, the tracked atol where the run tracks, and then no tighter than the weights the step was tested with), and the species add in quadrature (state_switch_band), which is what the WRMS test bounds: |Σ g_j e_j| ≤ √N·sqrt(Σ (g_j·u_j)²). The lanl#908 hold, which stands for half the test's band, reads it the same way. A residual of one species under a scalar or vector atol reads the same band to the bit; GH lanl#176's fixture and its five twins match main in steps, RHS, Jacobian and root counts and in the trajectory bytes at rtol 1e-6, 1e-8 and 1e-10, under a scalar and a vector atol. A residual of two or more terms gets a bar up to √n narrower, and the hold engages a little less on it. Cases 2 (√N over idle species) and 4 (the ×10 margin) are left alone.
Y = 5 is k over a window whose edges the model fixes, and a lost window is Y = 0. CVODE holds Y to rtol 1e-8 and the 30 runs are at most 3.1e-8 off (measured), so rel 1e-6 leaves 30x. The two tests now share the value and its derivation. Mutation check, each rebuilt and run: adding the band's terms as a plain sum fails the six case-3 runs over 1000 species that main fails, and ignoring the tracking atol fails the six case-5 runs. Dropping the floor at the step's error weights fails nothing: no test here exercises it.
…nl#907) Review of PR lanl#1329. No correctness defect found: the l2 band times 10*sqrt(N) is the Cauchy-Schwarz bound of CVODE's WRMS test, the hold's band stays under two thirds of the restart test's in every mode (the step-weight floor only widens the test's), and both call sites hand the test an N_VClone(y) scratch. The PR's 30 tests fail 12 of 30 against main's core, the ones it names. What changes here, none of it in what a run computes: - tolerance_unit re-implemented fill_tracking_ewt's clamp. The band's premise is that u_j is 1/ewt_j (the step-weight floor compares the two), so two hand-kept copies of the rule could drift silently. Both now read tracking_unit, the same expression, so the weights are the same bits. tolerance_unit's comment says that under tracking its rtol and atol_i arguments are not read. - StateSwitchHold held a nullable pointer to the run's tracking spec, with a static kNoTracking fallback for null. run() sets it wherever it sets user_data.state_switch_hold, so the fallback could not be taken. The hold now holds the spec by value, copied once per run. - Under tracking, each restart test cloned an N_Vector to receive the error weights. They now land in the test's own scratch, which nothing has read yet and whose contents the readings overwrite after, as before. - Hold creation evaluated tolerance_unit at x0 twice per species, for the box and for the gate's band. Once now. - The gate's comment said its band is half the restart test's "as it reads it". Under tracking the test floors each unit at the step's weights and the gate does not, so it is at most half: the side that keeps the band under the test's. Said so. - The test module's "What this locks" gains the lanl#907 windows, and the changelog's "One of one term" reads "A residual of one term". Left as they are: the floor applies only under tracking, though its reason (the state moves within the step) holds for the rtol*|x| term in every mode; reading CVODE's weights everywhere changes main's bars on every model, like lanl#907's cases 2 and 4. And the floor and the hold under tracking stay untested, for the reason the PR gives: there is no oracle for the sliver the floor moves the bar in.
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
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.
Part of #907: cases 5 and 3. Case 1 was #1097. Cases 2 and 4 are left, so this doesn't close the issue.
Problem
A plain run restarts CVODE at a state-switch root only where the crossing is resolved: the solver's interpolant must go past the surface by more than the band the tolerance leaves the residual, times 10·√N (#903). The band was
with atol_j read from the run's static tolerance, and the species summed one by one. Two of the issue's cases come from that. The review that found them kept its probes locally, so they were rebuilt here from the issue's descriptions: a window that opens where a residual crosses and that a clock (T′ = 1) closes 0.5 later,
r() = if((Obs > thr) && (Tobs < 1), k, 0), with exact Y = k·0.5 = 5.TrackingAtol(decades=6, ceiling=1e-8), CVODE holds S, about 1e-6, to 2e-14. The band read the 1e-8 ceiling, though, so a window 5e-9 wide in S read as unresolved.On main (aedadd8),
rtol = atol = 1e-8, five phases of the window:Fix
Each species' term in the band is now what CVODE's own error-weight rule holds it to at the state read, and the terms add in quadrature:
tolerance_unit(include/bngsim/atol_vector.hpp) gives u_j:fill_tracking_ewt(rtol·|x| + clamp(rtol·|x|, lo, hi)) under a tracking atol.It is evaluated at the state of each reading, as the old band was. Under tracking it is also floored at the weights CVODE tested the step with (
CVodeGetErrWeights). A tracking atol moves with the state, so where a species falls within a step, the rule at the reading's state alone could hold it tighter than CVODE did. Without tracking the atol floors both, and the rule alone is main's band.Quadrature is what the WRMS error test bounds: with ‖e‖_WRMS ≤ 1, |Σ_j ∂g/∂x_j·e_j| ≤ √N·sqrt(Σ_j (∂g/∂x_j·u_j)²). The old sum is the looser bound, which made an observable's band grow with its number of species.
state_switch_bandscales by the largest term, so no square under- or overflows, and a residual of one species reads |∂g/∂x|·u to the bit. A term that isn't a finite number gives the sum, as before.Used by:
flow_carries_state_switch): its readings, the root's band where the halvings stop, and each species' reach;shown()in the root function read the same way.The Forward sensitivity is silently wrong once a rate law makes the state slide along its switching surface (dS/damp = 1 against 0) #926 slide detector keeps its own band. It runs only in sensitivity runs, where the restart test and the hold do not.
Who else sees it: any residual with two or more nonzero terms (
A < B,A + B > c) now gets a bar up to √n narrower, so it can restart where main stepped on. A .net rate law that slides along its state switch runs up to 12x slower since #903 (60 s timeout where 6.7 s sufficed) #908's holds engage a little less on such residuals: the gate|g_root| < bandis harder to meet, and the band is narrower. A residual with one nonzero term reads the same bar to the bit.Not changed: √N over all species (case 2) and the ×10 margin (case 4). Both change what GH Four Linux-only test failures, found by #169's first whole-suite run: the GH #176 FD fallback dies at a second crossing, and a steady-state fixture is exactly singular under reference LAPACK #176's declined crossings rest on, and they are left for their own change, as cvode: sample the restart test's step halving toward the root as well (#907) #1097 said.
Results
The same table, this branch:
GH #176's fixture and its five twins step exactly as main. That is the split, ramp, latch, damped and moving variants of
_ltype_variant, at rtol 1e-6, 1e-8 and 1e-10 (atol = rtol, t ∈ [0, 150], 301 points), with the FD and the automatic Jacobian. All 36 runs are identical to main inn_steps,n_rhs_evals,n_jac_evalsandn_root_evals, and in the observables' bytes. The same holds with a vector atol of the same values: 36 of 36. "split" reads two species, so its band goes through the quadrature.Tests
python/tests/test_net_state_switch_stepping.pygains 30 tests, through a_clock_closed_windowbuilder:test_a_window_on_an_observable_over_many_species_is_integrated, n = 1000, 5 phases × t_end 20 and 50test_a_window_under_a_tracking_atol_is_integrated, 5 phases × t_end 20 and 50The 12 that fail on main are the lost phases above, at both run lengths. Whether main fails one depends on where the step that spans the window ends, which is the step-geometry caveat the issue raises about case 1's test. Hence the two run lengths.
Test audit
Checked against the test-writing rules Bill sent: each expected value's oracle, each tolerance's reason, and each test shown to fail against broken code. Commit cd3d777.
Other checks:
test_run_tests_sh.py::test_the_script_itself_runs_walk_up_dependent_tests_green, which fails the same way on main in this environment, because the venv has no jax, diffrax or roadrunner and the stand-in run reports their modules as skips.changelog.d/907.fixed.2.md, sits beside cvode: sample the restart test's step halving toward the root as well (#907) #1097's907.fixed.md.benchmarks/perf_ab.py --quick, main aedadd8 against this branch (21 models up to 150 species,odeandcodegen). No significant slowdown, and the solver's work counters are the same on all 42 rows. The geometric-mean ratios are 0.982 warm and 1.002 cold forode, and 0.980 warm and 0.995 cold forcodegen. Two of the 21 networks root a state switch in a rate law,oscillatory_system(1) andgene_expression(3), and both take main's steps exactly. None of the seven larger models in the full suite has one.Review
There was no second session to review this, so a separate review agent read the diff, read-only. It found no correctness bug. It confirmed:
Its low findings are all fixed here:
Not covered