headmyshoulder / odeint-v2

odeint - solving ordinary differential equations in c++ v2
http://headmyshoulder.github.com/odeint-v2/
Other
337 stars 102 forks source link

make_controlled factory bug (setting m_max_dt to eps_rel and m_eps_rel to default) #228

Closed ghostshadow closed 5 years ago

ghostshadow commented 6 years ago

While constructing a contolled_runge_kutta using make_controlled the applied controller_factory calles the constructor of contolled_runge_kutta with (abs_error, rel_error, stepper) instead of the expected (error_checker, step_adjuster, stepper), which leads to the error_checker beeing constructed with (eps_abs=abs_error, eps_rel=) and the step_adjuster beeing constructed with (max_dt=rel_error).

This error is also present in the official boost release! (https://svn.boost.org/trac10/ticket/13524#ticket)

A major side effect is, when a step size larger then rel_error is requested on the created controlled stepper, the step size adjuster forces the size of substeps to be rel_error wide, which for example forces a calculation of step size 0.2 with a rel_error of 1.0e-6 to be chopped into 200000 substeps, leading to horrible computation performance.

mariomulansky commented 5 years ago

I don't really follow. For controlled_runge_kutta this factory is being called: https://github.com/headmyshoulder/odeint-v2/blob/master/include/boost/numeric/odeint/stepper/generation/generation_controlled_runge_kutta.hpp which looks to me like it's doing the right thing? Could you provide a code snippet illustrating the described behavior?

ghostshadow commented 5 years ago

I am sorry, you are right, the description is not as clear as i must have thought.

Example

For a mechanics simulator i wrote, i needed controlled variants of different runge-kutta versions (dopri5, fehlberg78, etc.), which i created like this:

ode::make_controlled(absolut_error_threshold, relativ_error_threshold, ode::runge_kutta_dopri5<SystemState>())

using these includes:

//...
#include <boost/numeric/odeint/stepper/runge_kutta_dopri5.hpp>
#include <boost/numeric/odeint/stepper/generation/generation_runge_kutta_dopri5.hpp>
//... (other includes for the other variants)

namespace ode=boost::numeric::odeint;

Try of a better explanation

The problem with this is, make_controlled calls factory(abs_error, rel_error, stepper), with factory() being resolved via get_controller (e.g.: get_controller<runge_kutta_dopri5<...>> ) to the constructor of controlled_runge_kutta<>:

controlled_runge_kutta<>(
        const error_checker_type &error_checker = error_checker_type(),
        const step_adjuster_type &step_adjuster = step_adjuster_type(),
        const stepper_type &stepper = stepper_type()
) : m_stepper(stepper), m_error_checker(error_checker), m_step_adjuster(step_adjuster)
{ }

Which is problematic, since error_checker_type and step_adjuster_type are template types, and the correct defaults of error_checker=error_checker_type() and step_adjuster=step_adjuster_type() are overwritten by error_checker=abs_error and step_adjuster=rel_error respectively for this call. Resulting in m_error_checker to be constructed like:

default_error_checker(eps_aps=abs_error, eps_rel=<default>, a_x=<default>, ...)

and m_step_adjuster like:

default_step_adjuster(max_dt=rel_error)

I hope, this makes the problem a bit more clear.

ghostshadow commented 5 years ago

Oh, right, after a bit more consideration it appears the error is on my side for not also including "stepper/generation/generation_controlled_runge_kutta.hpp" and therefor not providing the specialized controller_factory for the controlled_runge_kutta.

In this case this is just a case of incomplete/hard to understand documentation. Please see if you can make this more clear in the documentation.