Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

run_simulation integrator support #26

Open
SebastianM-C opened this issue Jun 9, 2020 · 1 comment
Open

run_simulation integrator support #26

SebastianM-C opened this issue Jun 9, 2020 · 1 comment

Comments

@SebastianM-C
Copy link
Contributor

SebastianM-C commented Jun 9, 2020

The current run_simulation interface is used to create and solve the appropriate ODE/SDE problem based on the thermostat algorithm.

function run_simulation(s::NBodySimulation, args...; kwargs...)
if s.thermostat isa LangevinThermostat
calculate_simulation_sde(s, args...; kwargs...)
else
calculate_simulation(s, args...; kwargs...)
end
end

It internally calls calculate_simulation for ODEs, which uses dispatch on the algorithm type to choose between creating and solving an ODEProblem or SecondOrderODEProblem.
function calculate_simulation(s::NBodySimulation, alg_type=Tsit5(), args...; kwargs...)
solution = solve(ODEProblem(s), alg_type, args...; kwargs...)
return SimulationResult(solution, s)
end
# this should be a method for integrators designed for the SecondOrderODEProblem (It is worth somehow to sort them from other algorithms)
function calculate_simulation(s::NBodySimulation, alg_type::Union{VelocityVerlet,DPRKN6,Yoshida6}, args...; kwargs...)
cb = obtain_callbacks_for_so_ode_problem(s)
solution = solve(SecondOrderODEProblem(s), alg_type, args...; callback=cb, kwargs...)
return SimulationResult(solution, s)
end

The current implementation thus only allows only some some dynamical ODE solvers to be used with run_simulation since any other would use the ODEProblem fallback and fail.

One way to solve this would be via a trait, as indicated in the comment. An other option would be to select only some predefined well performing integrators in the union and indicate to users that they should manually construct and solve the problem for other algorithms.
And a simpler option would be to just use SecondOrderODEProblems for everything and remove the ODEProblem path.

I can start a PR if we decide upon a solution.

@ChrisRackauckas
Copy link
Member

And a simpler option would be to just use SecondOrderODEProblems for everything and remove the ODEProblem path.

That's probably the right thing to do. I think the benchmarks are demonstrating that it's pretty much always the case you want to do this. Unless you really really want to use a method for stiff ODEs, but we can make some SecondOrderODEProblem stiff ODE solvers (we should, there are plenty to implement), and the fallback should work anyways nowadays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants