CVODES pull request?

[Bob Carpenter]

I don't see a pull request for CVODES to review.
I see the branch:

feature/issue-262-CVODES

but comparing that to develop shows that there's a merge
conflict. Sebastian --- can you fix the merge conflict and make a pull
request to review?

Alternatively, is there something you wanted me to take
a look at before code reviewing a pull request?

- Bob

> On Apr 5, 2016, at 6:50 PM, wds15 <notifi...@github.com> wrote:
>
> So all tests are brought in line with at least 1E-6 abs+rel tolerance and I added a
>
> integrate_ode_cvodes
>
> function which takes a solver argument. Of course, we can quickly adapt to whatever naming convention we end up with (probable the _stiff naming).
>
> @bob-carpenter , @betanalpha a code review from one of you would be great. At the moment a high-level feedback would be very useful for me to shape this thing up for a pull request.
>
> There are still cpplint errors... will attack them soon.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub
>

[Michael Betancourt]

For reference, the CVODES code should be nearly identical to
the current CVODE code. Just some changes to the external
library calls.

> --
> 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]

Hi Bob!

I managed the conflict and initiated a pull. I thought a pull now would have been premature, but it probably does not matter.

So if you could take a look, that would be great!

Thanks,
Sebastian

[Bob Carpenter]

Thanks.

It's the easiest way to do the code review.

- Bob

[Bob Carpenter]

When there are too many lines of files displayed, GitHub gives
up. So I can't see the diffs here. I'll just dive into the
code and take a look and do this the old fashioned way because
I don't know how to deal with GitHub not allowing us to comment
because there are too many files.

One option might be to break this down into two commits, one
for the library by itself, and one for the actual ODE solver.

- Bob

[Ben Goodrich]

On Friday, April 8, 2016 at 5:02:37 PM UTC-4, Bob Carpenter wrote:

When there are too many lines of files displayed, GitHub gives
up.  So I can't see the diffs here.  I'll just dive into the
code and take a look and do this the old fashioned way because
I don't know how to deal with GitHub not allowing us to comment
because there are too many files.

One option might be to break this down into two commits, one
for the library by itself, and one for the actual ODE solver.

You can inspect the diff locally if you are on that branch. Such as

git diff --summary develop # to see the files that were changed
git diff develop stan/ # to exclude changes to lib/

Ben

[Sebastian Weber]

For reference:

The bulk of the CVODES code is in the files under stan/math/rev/arr/functor/:

- integrate_ode_cvodes.hpp : user-facing function
- cvodes_integrator.hpp : CVODES integrator interface
- ode_model.hpp : helper object to get easily the Jacobian of the ODE RHS function wrt to states and parameters
- decouple_states.hpp : helper which takes output from cvodes_integrator and creates with precomputed_gradient the output for integrate_ode_cvodes

The helper objects I had to introduce as I needed the mathematical objects a bit more granular than I would get it from the coupled_ode_system. It would make sense in going forward to refactor the coupled_ode_model object implementation using the two helpers I introduced, but that can be done at a later stage.

Sebastian