nathanaelbosch / ProbNumDiffEq.jl

Probabilistic Numerical Differential Equation solvers via Bayesian filtering and smoothing
MIT License
119 stars 16 forks source link

Calibration with `FixedDiffusion` and `FixedMVDiffusion` is broken with adaptive steps #273

Closed nathanaelbosch closed 10 months ago

nathanaelbosch commented 11 months ago

Currently the calibration is changed in each perform_step! - even in those that end up rejected! This leads to bad parameters. The most likely solution is to save an increment into the cache, and only commit it in DiffEqBase.savevalues!, which we overwrite in ./src/integrator_utils.jl already to similarly handle the way the state x is updated.

This issue should explain some of the behaviour we see in https://github.com/nathanaelbosch/ProbNumDiffEq.jl/pull/269; and fixing this issue should hopefully fix the calibration plots for these diffusion models and make the DynamicDiffusion and FixedDiffusion much more comparable.

nathanaelbosch commented 10 months ago

Actually the code seems to be correct after all! The global diffusion is updated only here:

https://github.com/nathanaelbosch/ProbNumDiffEq.jl/blob/fefbe97f9535a7afe1d867861406ae1ba1c4c34f/src/perform_step.jl#L124-L126

This only gets called after checking the local error estimate for being small enough, so this shouldn't be influenced by step rejections. Which leaves it open to what causes the bad calibration behavior in the benchmarks..