refactor coupled_ode_system

52 views
Skip to first unread message

Sebastian Weber

unread,
Jun 28, 2016, 2:54:12 PM6/28/16
to stan development mailing list
Hi!

As mentioned during the meeting, I would like to refactor the coupled_ode_system with the goal

- to get stiff and non-stiff ODE stuff aligned in the way they are implemented for easier maintenance

- make both (stiff+non-stiff) use the ode_system object introduced with CVODES patches

- this will enable to provide analytic Jacobians via partial template specialization to the stiff and non-stiff system at the same time

I started a branch here:

https://github.com/stan-dev/math/tree/feature/issue-310-coupled_ode_system-refactor

and the respective issue is this one:

https://github.com/stan-dev/math/issues/310

The implementation follows very closely the design of CVODES stuff, so I hope things are in a good state already. At this stage I was able to cut down the code by 500 lines, but lets wait for the review...

I suggest to do discussions on the issue unless we need a longer design phase, then we are probably better off with a wiki page.

From my end my only question would be if I should also remove the decouple_states function from the coupled_ode_system object. The decoupling operation could easily be moved into the integrate_ode_rk45 function (using the decouple_ode_states function).

If someone of the "ODE-code-exposed" persons could look into this, that would be great. Don't be scared by the many files moved around, this is only due to the fact that tests needed to relocate. The center-piece of the patch is in stan/math/rev/mat/functor/coupled_ode_system.hpp .

Many thanks!

Best,
Sebastian

Sebastian Weber

unread,
Jul 5, 2016, 4:40:58 PM7/5/16
to stan development mailing list
Hi!

So as I pointed out during the meeting I have now an easily readable version of the code:

https://github.com/stan-dev/math/blob/01dcd3a31daf4d3ff9eaca21817ae2d73dda1977/stan/math/rev/mat/functor/coupled_ode_system.hpp

which is about 3-5% or so slower than the old code. The version I just checked in should be just as fast as the old version, but the readability is a bit less good, I think:

https://github.com/stan-dev/math/blob/9d2d1f06378cdff6faa3fd072358985ee629fc8c/stan/math/rev/mat/functor/coupled_ode_system.hpp

Any comments greatly appreciated, maybe I start off a pull?

Sebastian

Sebastian Weber

unread,
Jul 5, 2016, 5:24:31 PM7/5/16
to stan development mailing list
Btw, i just benchmarked a non stiff ode problem once using the autodiff based jacobian approach against a version which uses analytic jacobians. This is possible to do with the refactoring i just did. The speedup is almost a full 2x speed increase! Quite cool.

Sebastian

Bob Carpenter

unread,
Jul 5, 2016, 8:45:25 PM7/5/16
to stan...@googlegroups.com
If you're done working on it, then by all means
submit a pull request.

I assume the 3--5% efficiency matters here, though
in general, I'm OK trading a bit of efficiency for
easier maintenance.

I'll start thinking about language issues for user-defined
Jacobians in general. We could have them for just about
any user-defined function. But then I don't know if that'll
fit in easily with the ODE stuff---we may have to do something
special for those functions.

- Bob

On Jul 5, 2016, at 2:24 PM, Sebastian Weber <sdw....@gmail.com> wrote:
>
> Btw, i just benchmarked a non stiff ode problem once using the autodiff based jacobian approach against a version which uses analytic jacobians. This is possible to do with the refactoring i just did. The speedup is almost a full 2x speed increase! Quite cool.
>
> Sebastian
>
> --
> You received this message because you are subscribed to the Google Groups "stan development mailing list" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+u...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Sebastian Weber

unread,
Jul 7, 2016, 2:53:23 AM7/7/16
to stan development mailing list
Hi!

Ok. You got the pull request. If design is deemed OK, then I owe you some tests for ode_system.jacobian as I had to generalize the possible arguments to that function (I will add them to the patch before it goes into math, of course). So please have a look at things. As I tried to stay close to previous design decisions, I hope that this one is an easier one to include.

Best,
Sebastian

On Wednesday, July 6, 2016 at 2:45:25 AM UTC+2, Bob Carpenter wrote:
> If you're done working on it, then by all means
> submit a pull request.
>
> I assume the 3--5% efficiency matters here, though
> in general, I'm OK trading a bit of efficiency for
> easier maintenance.
>
> I'll start thinking about language issues for user-defined
> Jacobians in general. We could have them for just about
> any user-defined function. But then I don't know if that'll
> fit in easily with the ODE stuff---we may have to do something
> special for those functions.
>
> - Bob
>
> On Jul 5, 2016, at 2:24 PM, Sebastian Weber
> >

Sebastian Weber

unread,
Jul 7, 2016, 1:53:21 PM7/7/16
to stan development mailing list
Hi Bob!

I forgot...you mentioned user specified gradients for user functions... Now that would be totally amazing, i think. If this is doable in resonable time, then that would be be awsome. Of course, the ode stuff would probably need something special, as always.

Best,
Sebastian

Reply all
Reply to author
Forward
0 new messages