JuliaDynamics / DynamicalSystemsBase.jl

Definition of dynamical systems and integrators for DynamicalSystems.jl
Other
56 stars 29 forks source link

Error checking with successful_step function #168

Closed rusandris closed 1 year ago

rusandris commented 1 year ago

Hi! I've added successful_step as part of the DynamicalSystemsBase API. For now it seemed like a good first step to implement it for ContinuousTimeDynamicalSystem type (it uses already available SciMLBase functions), but it can be extended to others as well, depending how general we want it to be. An older version of this PR was discussed at https://github.com/JuliaDynamics/DynamicalSystemsBase.jl/pull/147 and had the purpose to have an error checking tool that can be used to stop trajectory, lyapunov etc. calculations in case stepping is unsuccessful.

Datseris commented 1 year ago

Also, add the function to the API in the docstring of DynamicalSystem, and expand it to the docs as well. And increase version to 3.1.0 and add a new changelog entry !

rusandris commented 1 year ago

I've added the default and discrete methods, please check if they are in the right place. Also added docstrings and hopefully expanded to docs as well.

Is it possible to disable diffeq warnings like dt < dt min, etc.? Then we can throw our own warnings if we want to.

Yes, there is Supressor.jl but I don't know if that's a good idea :).

Datseris commented 1 year ago

Yes, there is Supressor.jl but I don't know if that's a good idea :).

Nah, that is not the way to go. I was thinking whether there was a keyword argument one gives when initializing an ODEIntegrator. But I guess you are right, there is no reason to suppress this really.

codecov-commenter commented 1 year ago

Codecov Report

Merging #168 (6dbab2d) into main (ab2007e) will decrease coverage by 0.07%. The diff coverage is 60.00%.

@@            Coverage Diff             @@
##             main     #168      +/-   ##
==========================================
- Coverage   82.95%   82.88%   -0.07%     
==========================================
  Files          14       14              
  Lines         698      701       +3     
==========================================
+ Hits          579      581       +2     
- Misses        119      120       +1     
Impacted Files Coverage Δ
src/derived_systems/parallel_systems.jl 93.97% <ø> (ø)
src/derived_systems/projected_system.jl 93.02% <ø> (ø)
src/derived_systems/stroboscopic_map.jl 85.71% <ø> (ø)
src/core/dynamicalsystem_interface.jl 75.60% <33.33%> (-1.32%) :arrow_down:
src/core_systems/continuous_time_ode.jl 85.18% <100.00%> (+0.27%) :arrow_up:
src/derived_systems/tangent_space.jl 95.95% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

rusandris commented 1 year ago

This being merged, do you need help with implementing this function into trajectory, lyapunov etc.?

Datseris commented 1 year ago

You can add this to lyapunov and stuff, yes, but what is the reason to add this to trajectory? The function already behaves as it should when the step is not successful.

rusandris commented 1 year ago

The function already behaves as it should when the step is not successful.

I used to and still get weird behaviour when for ex. trajectory meets and unstability. For continuous systems, the stepping doesn't stop, fills the REPL with the warning from SciML and it also gets stuck? Try this:

using DynamicalSystemsBase
using OrdinaryDiffEq

function unstable_sys(du,u,p,t)
           du[1] = u[1]
           du[2] = -u[2]
       return nothing
       end
u0 = [10.0, 10.0]
diffeq = (alg = Vern9(),abstol = 1e-9, reltol = 1e-9)

uns = CoupledODEs(unstable_sys, u0, nothing; diffeq)
trajectory(uns,200)

I suggest we use successful_step to fix this. Let me know what you think

Datseris commented 1 year ago

Of course; we didn't actually use successful_step anywhere, so it makes sense that the behavior of trajectory didn't change. So yes I agree you should do a PR That uses successful_step in trajectory! I'm already using succesful_step! in other parts of the library!