interpolation and saving support - #107
Merged
Merged
Conversation
Saving and events are implemented on `OperatorSplittingIntegrator` only. The inner
splits are stages rather than steps, so their intermediate states do not approximate
the split solution at any time point and there is nothing meaningful to save or to
test a condition against there. This is enforced structurally rather than by
convention: the saving options and `saveiter` are fields of the outer integrator
alone, consumed as plain `__init` keywords so they never enter the `ConfigTree` and
cannot leak to the leaves, and `save_step!`/`handle_callbacks!` are no-ops on a
`SplitSubIntegrator`.
Interpolation
`(integrator)(t, Val{D}; idxs)` and `(integrator)(val, t, Val{D}; idxs)` now follow
the SciML contract, delegating to a cache-dispatched `splitting_interpolant[!]` whose
fallback is linear. A higher-order dense output is therefore one method per
algorithm, with no changes to saving or callbacks. Note that Θ is derived from
`t - tprev`, never from `integrator.dt`: after an accepted step
`step_accept_controller!` has already replaced `dt` with the next proposal.
`change_t_via_interpolation!` refills `u` from the interpolant and re-anchors the
whole subintegrator tree through `rollback_children!`. It decodes
`modify_save_endpoint` by dispatch because DiffEqBase passes the literal
`Val{:false}` -- a `Val` of the Symbol -- which `if T` would choke on.
Saving
`SciMLBase.savevalues!` is the single integration point; DiffEqBase's callback code
calls exactly that generic. `saveat`, `save_everystep`, `save_start`, `save_end` and
`save_on` are supported. `saveat` points inside a step are interpolated, so asking
for output never changes the step sequence and therefore never changes the splitting
error. `dense = true` is rejected: the interpolant is linear, so `sol(t)` already
reproduces it exactly from the saved points.
Callbacks
`DiscreteCallback`, `ContinuousCallback`, `VectorContinuousCallback` and
`CallbackSet` all work, with `save_positions`, `affect_neg!`, `rootfind`,
`interp_points`, `terminate!` and `derivative_discontinuity!`. The root finding and
`affect!` dispatch are DiffEqBase's; this adds the per-step driver
`handle_callbacks!`, the generated `apply_ith_callback!`, `initialize_callbacks!`,
and the integrator-side methods DiffEqBase requires.
The splitting-specific hazard is child state: every child owns a copy of its slice of
`u`, so an `affect!` that modifies `integrator.u` would leave the tree stale, and the
palindromic schemes skip the forward sync of their first child so it would not
self-correct. `reeval_internals_due_to_modification!` runs `rollback_children!`,
which both propagates the new state and restores the `parent.u == child.u` invariant
that shortcut depends on.
Bug fixes
- `init(prob, alg; callback = cb)` threw `FieldError`: DiffEqBase reads
`derivative_discontinuity` as a literal field, and this package named it
`u_modified` while defining only the setter. Callbacks were unusable.
- `postamble!` ran once per step, so callback `finalize` hooks fired every step.
SciMLBase's generic `check_error!` finalizes whenever the code is not `Success`,
and a mid-solve node legitimately reports `Default`; `check_error!` is now
overridden and `postamble!` made idempotent.
- `reinit!(integrator, u0)` silently returned wrong results for `StrangMarchuk`. A
leaf's `reinit!` restores the `u0` slice captured when it was built, and Strang's
skipped first sync meant the stale child state was integrated. `reinit!` now
pushes the new state down explicitly, and initializes callbacks after the child
reinit so a modifying initializer is not undone.
- `reinit!(...; reinit_callbacks = false)` threw `BoundsError` indexing an empty
`discrete_callbacks` tuple for a saving callback that never existed.
- `handle_tstop!` called `change_t_via_interpolation!`, for which no method existed.
- The `saveat` heap included `t0` and duplicated `tf`.
- Removed dead `fix_solution_buffer_sizes!`, which referenced fields the integrator
does not have.
Accuracy limits, documented rather than left to be discovered: interpolated output
and located event times are second order accurate, an event is the exact root of the
linear interpolant over the bracketing step (so cap `dtmax` when event accuracy
matters), and an event that reverses within a single step cannot be detected --
which also means raising `interp_points` cannot help.
The SciMLBase lower bound moves to v3, matching the `derivative_discontinuity` field
name. Every DiffEqBase 7.x already required SciMLBase 3.x, so the previous
`2.77.0` entry was unreachable in practice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cleanup pass over the saving, interpolation and callback support, with no
intended behavior change.
Drop duplicated upstream code:
- `apply_ith_callback!` was a byte-for-byte copy of
`OrdinaryDiffEqCore.apply_ith_callback!`, which is generic in `integrator`.
Call upstream's and add it to the QA ignore list.
- `save_initial_value!` re-inlined `_save_current!` with a hardcoded index.
- The two save branches of `savevalues!` now share one `_save_at!` writer.
Drop compat shims made dead by the SciMLBase 3.1 bump: the `@static` pairs for
`u_modified!` / `derivative_discontinuity!` collapse to single unconditional
methods. The `DEVerbosity` shims stay, since they are gated on DiffEqBase.
Remove needless indirection:
- `resync_children_after_modification!` was a pure alias for
`rollback_children!`; its three call sites now go direct.
- `_modify_save_endpoint`'s dispatch pair becomes one identity comparison,
which tolerates DiffEqBase's `Val{:false}` just as safely.
- `_step_dt` and `_interp_theta` merge into `_interp_coords`, so the step
length is computed once per interpolation instead of twice.
- Rename the local `solution_endpoint_match_cur_integrator!` to
`save_endpoint!`: it shadowed an unrelated OrdinaryDiffEqCore function of
the same name.
Avoid work in the step loop: `savevalues!` returns early when there is nothing
to save, and building the `saveat` heap no longer allocates twice.
Finally, the "inner splits are stages, not steps" argument was written out
eight times across src/ and the docs. It now lives once in the devdocs "Dense
output" section, with one-line references from the code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closed
Collaborator
|
Downgrade CI needs a fix. LGTM other than that. |
termi-official
approved these changes
Aug 4, 2026
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
2 participants
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.
supports callbacks and saving (with a pretty bad linear interpolation)