Using new CoupledSDEs

[reykboerner]

Replace the CoupledSDEs type defined in the package with the new CoupledSDEs type loaded from DynamicalSystemsBase v 3.11.

This will be a breaking change in the core type of the package.

Todo's

• The action function give much larger numbers than before. Check what going on there.
• Sampling trajectories should use the DynamicalSystemsBase.trajactory framework
• remove relax?

[oameye]

@reykboerner I set all the tests that ckeck the action value to broken. The reason whu it is so large is because the inverse of the the covariance matrix. Which for a noise strenght σ=0.2 which has a diagonal covariance matrix of value σ^2=0.04. Doing inv(cov)=1/σ^2=25 gives high numbers. I don't know what we were doing before.

[reykboerner]

@reykboerner I set all the tests that ckeck the action value to broken. The reason whu it is so large is because the inverse of the the covariance matrix. Which for a noise strenght σ=0.2 which has a diagonal covariance matrix of value σ^2=0.04. Doing inv(cov)=1/σ^2=25 gives high numbers. I don't know what we were doing before.

Before, we weren't including the scalar σ in the covariance matrix. The FW action value should be independent of σ and only depend on the relative magnitudes of the entries C_ij in the covariance matrix. I have therefore added a step to normalize the covariance matrix before doing inv(cov). Now the tests work again :)

[reykboerner]

Regarding simulation, there are now three functions:

• trajectory(sys::CoupledSDEs) imported from DynamicalSystemsBase
• simulate(sys::CoupledSDEs), a wrapper around solve(prob::SDEProblem) for uses beyond the simple usage of trajectory (e.g. using callbacks) - note: also naming this function solve would be type piracy
• relaxation(sys::CoupledSDEs), which is basically just a shortcut for trajectory(CoupledODEs(sys::CoupledSDEs)), so one could debate if it's needed. Could be convenient for quickly running deterministic trajectories, though.

What do you think of this and the naming @oameye @ryandeeley @raphael-roemer ?

[reykboerner]

* [x]  Sampling trajectories should use the `DynamicalSystemsBase.trajactory` framework

Since transition uses callbacks, I think it should use the solve(SDEProblem) framework instead of DynamicalSystemsBase.trajectory. This is how I updated it now

[raphael-roemer]

Regarding simulation, there are now three functions:

• trajectory(sys::CoupledSDEs) imported from DynamicalSystemsBase
• simulate(sys::CoupledSDEs), a wrapper around solve(prob::SDEProblem) for uses beyond the simple usage of trajectory (e.g. using callbacks) - note: also naming this function solve would be type piracy
• relaxation(sys::CoupledSDEs), which is basically just a shortcut for trajectory(CoupledODEs(sys::CoupledSDEs)), so one could debate if it's needed. Could be convenient for quickly running deterministic trajectories, though.

What do you think of this and the naming @oameye @ryandeeley @raphael-roemer ?

Sounds good to me. I think it is good to have an extra name for "relaxation" to make it a bit clearer that it does not take into account the stochastic part of the equation. However, calling it "det_rajectory" or something similar might make it even more obvious.

[reykboerner]

However, calling it "det_rajectory" or something similar might make it even more obvious.

Yeah, I also don't find relaxation ideal. deterministic_orbit would be another possibility.

[ryandeeley]

1. I agree that deterministic_orbit would be a better name for the function currently called relax. However, I also agree that it is not necessary if one can always use trajectory(CoupledODEs(sys::CoupledSDEs)).
2. Certainly many functions use callbacks, so I would suggest that these functions call the solve(prob::SDEProblem) once an appropriate prob::SDEProblem has been created in the background that is in line with what is desired from the original function. For the most part, I would say we can name these functions as we had them before in CriticalTransitions.jl, however since we now have an additional trajectory function from DynamicalSystems.jl, I would find it a little strange having two functions with separate names (trajectory(sys::CoupledSDEs) and simulate(sys::CoupledSDEs)) that can return the same thing. Can we use multiple dispatch here and have the same function name for two different collection of arguments (one including a term cb::DiscreteCallback for instance, which will use solve(prob::SDEProblem) rather than trajectory(sys::CoupledSDEs), and essentially acting as the original simulate(sys::StochSystem) function just now called trajectory(sys::CoupledSDEs).)? Or would this be whatever type piracy is?

[reykboerner]

2. Or would this be whatever type piracy is?

Yes, trajectory(CoupledSDEs) would be type piracy because neither the function nor the type come from CT.jl. Note that trajectory from DynamicalSystems is just a convenience wrapper around solve, as mentioned in the docstring of trajectory. So what we want essentially is solve(CoupledSDEs).

We could have a function SDEProblem(CoupledSDEs) and then use solve(SDEProblem(CoupledSDEs)).

2. two functions with separate names (trajectory(sys::CoupledSDEs) and simulate(sys::CoupledSDEs)) that can return the same thing

They return slightly different things. trajectory returns a StateSpaceSet and simulate returns the output type of solve(SDEProblem).

[ryandeeley]

Yes, trajectory(CoupledSDEs) would be type piracy because neither the function nor the type come from CT.jl.

Okay, so how would we resolve this without moving either the function or the type into CriticalTransitions.jl?

We could have a function SDEProblem(CoupledSDEs) and then use solve(SDEProblem(CoupledSDEs)).

And would this overcome the type piracy? I don't think I really understand it.

They return slightly different things. trajectory returns a StateSpaceSet and simulate returns the output type of solve(SDEProblem).

Okay, yes, I see this, although it wasn't really what I was trying to say. I meant we use multiple dispatch to allow trajectory to either take in a callback::DiscreteCallback as an argument or not, because currently you cannot pass a callback::DiscreteCallback argument to the trajectory function if I understand correctly. I think however that both versions should return the same thing, be that either a StateSpaceSet or an output of type solve(prob::SDEProblem).

[reykboerner]

And would this overcome the type piracy? I don't think I really understand it.

The problem would be solved if we define the function in DynamicalSystemsBase directly.

@Datseris what do you think of including a wrapper DynamicalSystemsBase.solve(ds::CoupledSDEs, T [, u0]; kwargs...) that extracts the SDEProblem from ds and then offers the full functionality of solve from DiffEq, including callbacks etc.? This would be in addition to trajectory.

[Datseris]

Hi, trajectory(CoupledSDEs) should work out of the box. It doesn't?

[Datseris]

There is a long discussion I haven't been part of, can someone please summarize the issue you have, not the solution you propose, so I can give proper advice? (I won't have the time to back trace the whole discussion)

[reykboerner]

Hi, trajectory(CoupledSDEs) should work out of the box. It doesn't?

It works perfectly but the docstring says it is only a simple wrapper that does not work well with callbacks for continuous systems.

So the issue is that we would like to have a function that offers the full functionality of DynamicalSystems.solve for a CoupledSDEs (which is an SDEProblem behind the scenes). If we define such a function in CriticalTransitions.jl, it would be type piracy.

[Datseris]

I should improve the docstring as it actually isn't a wrapper of solve...

I think it is fine if you extend solve here. Yes, formally it is type piracy, but I think in this case it is very safe as the same developer team develops the CoupledSDEs.

What situations do you need solve for?

[reykboerner]

What situations do you need solve for?

We need solve for callbacks when we sample transition paths, but actually we don't need to extend it for the end user for now.

So I have removed simulate (which was the wrapper around solve) and renamed relaxation -> deterministic_orbit based on the comments above. End users can use the out-of-the-box trajectory function.