Skip to content

test: cap dt in the marginal continuous-callback crossing-time tests - #4866

Merged
AayushSabharwal merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:test/tighten-marginal-continuous-callback-solves
Aug 4, 2026
Merged

test: cap dt in the marginal continuous-callback crossing-time tests#4866
AayushSabharwal merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:test/tighten-marginal-continuous-callback-solves

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member

Please ignore this PR until it has been reviewed by @ChrisRackauckas.

Fixes the three Additional SymbolicContinuousCallback options failures currently on master (e.g. https://github.com/SciML/ModelingToolkit.jl/actions/runs/30746352714/job/91493644801, reproducing on julia 1, lts and pre):

symbolic_events.jl:729 / :846 / :866
  Expression: maximum(abs.(first.(cr2) .- required_crossings_c2)) < 0.0001
   Evaluated: 0.00026350432675315005 < 0.0001

Why these tests were failing

They were marginal by construction. All three assert event times to 1.0e-4 while integrating solve(prob, Tsit5()) at the default reltol = 1.0e-3. For c2 = cos(3t) that is not enough headroom: the numerical trajectory carries ~2e-4 of error, and since dc2/dt = ±3 at the crossings the recorded event time inherits error/3 ≈ 7.3e-5 — a 1.4× margin against the assertion.

The rootfind was never the problem. The condition value recorded at the event time is ~1e-16, i.e. the interpolant's zero is found exactly; what these assertions were actually measuring was default-tolerance quadrature error, not event handling.

What tipped them over

SciML/OrdinaryDiffEq.jl#3720, released in OrdinaryDiffEqCore 4.12.0, reinitializes the step-size controller after any continuous callback with maybe_discontinuity = true:

if _fired_cb_maybe_discontinuity(idx, continuous_callbacks)
    reinit_controller!(integrator, integrator.controller_cache)
end

That is the right thing to do — a PI controller's error history assumes the dynamics before and after the step are the same, which is exactly what a discontinuity breaks — but it changes the accepted step sequence. On this problem the c2 error grew from ~2.2e-4 to ~7.9e-4, so the crossing times moved from 7.3e-5 to 2.6e-4 and crossed the threshold.

Bisected on a minimal, MTK-free reproducer (plain ODEProblem + VectorContinuousCallback with the same settings MTK generates):

OrdinaryDiffEqCore steps max c2 event-time error
4.0.0 – 4.11.0 31 7.3234e-5
4.12.0 34 2.6350432675e-4 ❌ (bit-identical to CI)
4.12.0, maybe_discontinuity = false 31 7.3234e-5

The change

Pass dtmax = 0.01, exactly as the other four solve calls in the same testset already do — those blocks land within ~1e-12 and were completely unaffected by the upstream change. The 1.0e-4 assertions are left alone; with dtmax capped the crossing times come out at ~1e-12, so they now test the rootfind instead of the controller.

Testing

Ran the full file locally against this branch with ModelingToolkit loaded (mirroring the failing InterfaceI job), OrdinaryDiffEqCore 4.12.0 / DiffEqBase 7.10.0 / SciMLBase 3.41.0, Julia 1.12.6:

Additional SymbolicContinuousCallback options |   34     34  16.0s
...
EXITCODE=0

31 testsets, no failures, no errors. On the same environment before the change, the three assertions fail with the exact CI values. Runic is clean.

The three `solve(prob, Tsit5())` calls in "Additional
SymbolicContinuousCallback options" assert event times to 1.0e-4 while
integrating at the default `reltol = 1.0e-3`. For `c2 = cos(3t)` that
leaves no room: the numerical trajectory carries ~2e-4 of error, and
since `dc2/dt = ±3` at the crossings the recorded event time inherits
~7e-5 of it -- a 1.4x margin against the assertion. The rootfind itself
is exact (the condition value at the recorded time is ~1e-16), so what
these tests actually measured was default-tolerance quadrature error,
not event handling.

OrdinaryDiffEqCore 4.12.0 (SciML/OrdinaryDiffEq.jl#3720) reinitializes
the step-size controller after any continuous callback with
`maybe_discontinuity = true`, which is correct -- a PI controller's
history is meaningless across a discontinuity -- but it changes the step
sequence, and the c2 error grew to ~8e-4, pushing the crossing times to
2.6e-4 and over the threshold.

Pass `dtmax = 0.01` as the other four solves in the same testset already
do. Crossing times land within ~1e-12, so the assertions now test the
rootfind rather than the controller.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TNqmRywBbj51czPQgoiEC7
@AayushSabharwal
AayushSabharwal marked this pull request as ready for review August 4, 2026 15:19
@AayushSabharwal
AayushSabharwal merged commit 71e817e into SciML:master Aug 4, 2026
82 of 92 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants