Skip to content

Recompute non-lazy interpolation stages after a callback-truncated step#3964

Merged
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:fix-addsteps-truncated-step-nonlazy
Jul 20, 2026
Merged

Recompute non-lazy interpolation stages after a callback-truncated step#3964
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:fix-addsteps-truncated-step-nonlazy

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member

Fixes #3963.

Please ignore until reviewed by @ChrisRackauckas.

Root cause

reeval_internals_due_to_modification! rebuilds the interpolation stages after a ContinuousCallback truncates a step. The force_calc_end argument it passed to ode_addsteps! could never be true:

if SciMLBase.has_lazy_interpolation(alg)
    ode_addsteps!(integrator, integrator.f, true, false, !_unwrap_val(alg.lazy))
else
    ode_addsteps!(integrator, integrator.f, true, false)
end

has_lazy_interpolation(alg) is defined as _unwrap_val(alg.lazy) (OrdinaryDiffEqVerner/src/alg_utils.jl:18), so inside the true branch !_unwrap_val(alg.lazy) is always false, and the else branch takes the false default. Dead logic on both paths.

Why that only breaks lazy=false:

  • lazy=truekshortsize excludes the extra stages (13 for Vern8), so the preceding resize! drops them and lazy interpolation rebuilds them against the new dt. Correct.
  • lazy=falsekshortsize includes them (21 for Vern8), so resize! is a no-op and stages 14–21 survive holding values computed for the original, longer dt. _ode_addsteps! then skips them, because its guard (allow_calc_end && length(k) < 21) || force_calc_end is false on every term.

Step endpoints stay correct since they derive from stages 1–13 — only the interior interpolant of the truncated step is wrong, exactly as reported.

Scope beyond the issue

BS5(lazy=false) has the identical defect (kshortsize 11 vs 8, guard at low_order_rk_addsteps.jl:175). Not mentioned in the issue; verified below.

Verification

MRE from the issue, before → after:

algorithm truncated (before) truncated (after) prev. clean
Vern8() lazy 4.94e-11 4.94e-11 6.52e-11
Vern8(lazy=false) 1.64e-02 4.94e-11 6.52e-11
Vern9(lazy=false) 4.44e-03 1.56e-13 2.04e-13
Vern7(lazy=false) 7.24e-04 6.10e-13 6.18e-13
Vern6(lazy=false) 5.44e-04 2.43e-12 2.81e-12
Tsit5() 6.63e-12 6.63e-12 1.18e-11

Vern8(lazy=false) now reads 4.94e-11, identical to Vern8() lazy — that equality is what the bug was breaking. The prev. clean column is unchanged and the lazy/Tsit5 rows are bit-identical, so untruncated steps and non-Verner algorithms are unaffected.

Negative controls (tests run against the unfixed tree, to confirm they actually catch the bug):

Verner suite, no fix:   35 pass, 4 fail  <- exactly the four lazy=false variants
Verner suite, with fix: 39 pass
BS5 test,     no fix:   FAILS, 9.25e-5
BS5 test,     with fix: 2/2 pass
LowOrderRK suite:       70 pass

The Verner failures partition exactly along the lazy boundary the analysis predicts, and the other 35 tests pass on the unfixed base — so there are no pre-existing failures on this commit.

Caveats

  • Verified locally on Julia 1.12.6: ODEDIFFEQ_TEST_GROUP=Core for OrdinaryDiffEqVerner and OrdinaryDiffEqLowOrderRK only. Not the full monorepo suite, not QA.
  • The change makes force_calc_end true for every non-lazy algorithm, not just Verner/BS5. Only those two families have a force_calc_end branch in _ode_addsteps! at all, and all methods share the 5-arg signature via the generic ode_addsteps! (generic_dense.jl:109), so it should be inert elsewhere — but that part rests on inspection rather than execution, and is what CI should confirm.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Sc5xC1E4H646cPhLSccGCP

When a ContinuousCallback truncates a step, reeval_internals_due_to_modification!
rebuilds the interpolation stages for the shortened dt. The force_calc_end
argument it passed to ode_addsteps! could never be true: has_lazy_interpolation
is defined as _unwrap_val(alg.lazy), so the guarded !_unwrap_val(alg.lazy) was
false whenever the guard was taken, and the else branch used the false default.

This was harmless for lazy interpolants, whose extra stages sit outside
kshortsize and are therefore dropped by the preceding resize! and rebuilt on
demand. For non-lazy interpolants the extra stages sit inside kshortsize (21 vs
13 for Vern8, 11 vs 8 for BS5), so resize! cannot drop them and _ode_addsteps!
skipped them, leaving values computed for the original longer dt. Step endpoints
stayed correct because they come from the earlier stages; only the interior
interpolant of the truncated step was wrong.

Affects Vern6/7/8/9 with lazy=false and BS5 with lazy=false.

Fixes SciML#3963

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review July 20, 2026 11:34
@ChrisRackauckas
ChrisRackauckas merged commit 38f01c5 into SciML:master Jul 20, 2026
219 of 254 checks passed
ChrisRackauckas added a commit that referenced this pull request Jul 21, 2026
Releases unreleased changes on master across four sublibraries:
- OrdinaryDiffEqCore 4.7.1 -> 4.8.0 (minor: restores isdiscretecache public API (#3907); also recompute non-lazy interpolation stages after a truncated step (#3964))
- OrdinaryDiffEqBDF 2.3.1 -> 2.3.2 (precompile a mass matrix DAE in the workload (#3966))
- OrdinaryDiffEqExtrapolation 2.3.0 -> 2.3.1 (drop stale BaseThreads/Sequential imports (#3945))
- OrdinaryDiffEqRosenbrock 2.4.1 -> 2.4.2 (SIMD-fuse RosenbrockCache stage loops; document public HybridExplicitImplicitRK (#3938))



Claude-Session: https://claude.ai/code/session_01SFmcAmLrGPrGtzwP333mX8

Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

The returned interpolant is wrong inside a step truncated by a ContinuousCallback for certain algorithms

2 participants