SciMLBase.jl
SciMLBase.jl copied to clipboard
SciML
Upgrade path for check_error interface conformity
This is the first step for https://github.com/SciML/SciMLBase.jl/pull/896 (see https://github.com/SciML/SciMLBase.jl/pull/896#discussion_r1895057885). With this change I can as next step go to the downstream packages and adjust the last_step_failed function, and then I can go back to 896. (If we just merge 869 almost all CIs fail).
The larger context is to simplify the development of custom time integrators compatible which are at least partially compatible with the SciML ecosystem.
If the steps failed and the Newton algorithm diverged, I think we'd still error out unless the system had a bool set to just continue?
Right. However, this error is ~~thrown~~ handled in a different code path. This PR merely hoists the condition out of the function call. See e.g. https://github.com/SciML/OrdinaryDiffEq.jl/blob/013c00d81292d7ddce87841b7765dde030c2875e/lib/OrdinaryDiffEqCore/src/integrators/integrator_utils.jl#L34-L36 . This function is copy pasted across the all packages with custom integrators in SciML org.
If the steps failed and the Newton algorithm diverged, I think we'd still error out unless the system had a bool set to just continue?
Reading over it again, maybe I misunderstood you here. Right now in OrdinaryDiffEqCore the integrator has do_error_check, which is set to true in each iteration header. I cannot find the mentioned code path in your comment. I.e. I do not see how the Newton convergence failure for adaptive time integration algorithms results in the ConvergenceFailure flag being set for the integrator.
https://github.com/search?q=org%3ASciML%20ConvergenceFailure&type=code
I still don't understand what this is for. @oscardssmith should we talk about this? Is there a test that should go with this?
The preceding comment why I hoisted this out is here https://github.com/SciML/SciMLBase.jl/pull/896#discussion_r1895057885