Initial attempt at documenting new ODE interface (design-doc #19) by bbbales2 · Pull Request #213 · stan-dev/docs

bbbales2 · 2020-07-04T01:05:20Z

Submission Checklist

Builds locally
Declare copyright holder and open-source license: see below

Summary

Here's a rough attempt at rewriting the ODE docs for the new interfaces.

@wds15 add this to your list to review. The user's guide portion is pretty rough. I don't think we should plan on doing anything more than a compatibility rewrite before the next release, but I think we should rewrite this shortly.

Issues:

There's a missing plot currently. I have a pull req. to remake it here: Missing plot in ODE section of user's guide #160 , and but the plot appears to be as a repo as a .pdf but that's not rendering on the web (here). Can I just convert it to a .png?
We can get rid of the old interface stuff in the user's guide, and it doesn't take up much space in the language reference, however, I think we should keep the full docs around in the function's guide since even though this is a deprecated feature there are going to be lots of models out there using integrate_ode_X functions. How do we demarcate this in the function guide? Presumably we need to make it easy for people to find that page, but also easy once they've found that page to realize that they should be using the new interfaces (on a different page).

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company): Columbia University

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

bbbales2 · 2020-07-04T01:06:48Z

@mitzimorris adding you here. I don't think you need to review the text so much (let's let @wds15 take a pass or two first), but do you have suggestions on #1 and #2 above?

SteveBronder

idk anything about ODEs but thought I'd help give some comments. The info is v good but I think the structure needs changed a bit

SteveBronder · 2020-07-06T22:29:48Z

+`...` are any number of arguments passed through unmodified from the ODE integrate
+call.
+
+The ODE system function should return the derivative of the state with
+respect to time at the time and state provided. The length of the returned
+vector must match the length of the state input into the function.
+
+The arguments to this function are:
+
+*   *`time`*, the time to evaluate the ODE system
+
+*   *`state`*, the state of the ODE system at the time specified
+
+*   *...*, all other arguments passed through from the integrate call. Each
+argument can be any type.


Before this section I would show an example signature for the ODE integrators because

all other arguments passed through from the integrate call

Doesn't really make sense without knowing what the integrators look like

Suggested change

`...` are any number of arguments passed through unmodified from the ODE integrate

call.

The ODE system function should return the derivative of the state with

respect to time at the time and state provided. The length of the returned

vector must match the length of the state input into the function.

The arguments to this function are:

* *`time`*, the time to evaluate the ODE system

* *`state`*, the state of the ODE system at the time specified

* *...*, all other arguments passed through from the integrate call. Each

argument can be any type.

Where the arguments to the function are:

* *`time`*, the time to evaluate the ODE system

* *`state`*, the state of the ODE system at the time specified

* *...*, Any additional arguments needed in the ODE. Each of these can be arguments of any type.

The ODE system function should return the derivative of the `state` with

respect to `time` at the `time` and `state` provided. The length of the returned

vector must match the length of the state input into the function.

I moved the ODE system function definition below the signatures and the arguments to the integrator functions.

It's still slightly confusing because the first argument to the integrator function is the ODE system function, but there's a little inline doc there that hopefully works.

SteveBronder · 2020-07-06T22:35:50Z

+*   *`ode`*: function literal referring to a function specifying  the
+system of differential equations with signature:


What does function literal mean? Should we use that same term when talking about the ODE function above?

Is this section the same as the one above?

Is this section the same as the one above?

I left a copy of the old integrator docs there. The function signature will be deprecated but I figure it should still have docs so people know how to convert old models. So there is duplicate information.

bbbales2 · 2020-07-07T01:40:32Z

Thanks @SteveBronder , those were useful comments. I resolved most of the simpler things and left open the big things if you wanna check them.

wds15 · 2020-07-07T11:19:27Z

Beginning of the stan user guides for odes it says

"Stan provides a built-in mechanism for specifying and solving systems of ordinary differential equations (ODEs). Stan provides three different integrators, tuned for solving non-stiff systems and for stiff systems."

can you please change that to

"Stan provides a built-in mechanism for specifying and solving systems of ordinary differential equations (ODEs). Three different integrators, tuned for solving non-stiff systems and for stiff systems are available."

wds15 · 2020-07-07T11:23:52Z

Also here:

docs/src/stan-users-guide/odes.Rmd

Line 19 in f145344

For a discussion of stiff ODE systems, see the [stiff ODE

It states that stiff solver are slower, but more robust. Well... that's not quite right. For a non-stiff system the stiff solver will be slower than the non-stiff one, but whenever a system is stiff, then the non-stiff solver will be much slower than the stiff solver. If you could clarify that, this would be great. So for non-stiff problems the non-stiff solvers shine and leave behind the stiff solvers by a large margin, but if the problems are stiff, then the reverse is true.

wds15

Just some cleanups to be done.

wds15 · 2020-07-07T11:29:06Z

 region of parameter space.  The function signatures for Stan's ODE
 solvers can be found in the reference manual section on ODE solvers.

-


It appears to me that we went with the convention to say $y(t)$, but this actually meant $y(t, \theta)$. So the parameters were dropped when we referred to y as a function of time. Can we either add everywhere $\theta$ as an argument to y or make it explicit in the introduction that we intentionally drop $\theta$ when referring to $y$? Thanks.

I agree. The fact that the rest of the ODE literature is sloppy about explaining their conventions doesn't mean we have to be! That $y$ is also a function of the data if we want to get really explicit.

wds15 · 2020-07-07T11:33:00Z

-parameters.  A full example for the simple harmonic oscillator, which
-does not depend on any constant data variables, is provided in the
-simple harmonic oscillator trajectory plot. 
+Every ODE system function has the same first two arguments and the


this section is confusing here as the ode_* calls have not been defined yet. You could include in 1.2 a paragraph saying how one gets from the coded ODE RHS, the solution $y$. Then it would be clear what you are talking about in this section.

Or at least cross-reference.

wds15 · 2020-07-07T13:25:07Z

+
+*   solution times and number of vectors in the output,
+
+## Old integrate_ode_X ODE Interface {#old-functions-ode-solver}


Maybe call this section "Deprecated integrate_ode_* ODE Interface" ?

It's probably best to start an appendix with deprecated functions as we do in the reference manual.

Better yet, move to an appendix the same way we do in the reference manual. Doc for deprecated function can be abbreviated.

bob-carpenter · 2020-07-07T17:14:13Z

Just convert .pdf to .png and it should work on web and in the pdf. You have to update the .Rmd to link to the new file.
The reference manual should document the non-deprecated features. There's an appendix for deprecated features, which should include all deprecated features that are still active. I then think we should do the same with the functions reference.

bob-carpenter

Done. Now you have two reviews.

bob-carpenter · 2020-07-07T17:18:31Z

+<!-- vector[]; ode_rk45; (function ode, vector initial_state, real initial_time, real[] times, ...); -->
+\index{{\tt \bfseries ode\_rk45 }!{\tt (function ode, real[] initial\_state, real initial\_time, real[] times, ...): vector[]}|hyperpage}
+
+`vector[]` **`ode_rk45`**`(function ode, vector initial_state, real initial_time, real[] times, ...)`<br>\newline


The ... thing really needs to be explained up front. I was reading along assuming it was just tolerances or something like that and I find out it's the whole new way of dealing with ODEs. This needs to be said up front!

bob-carpenter · 2020-07-07T18:01:18Z

+```
+
+In general it is better to integrate up to `t_lag` in one solve and
+then integrate from `t_lag` onwards in another.


Cool. I was wondering about that. Definitely include this.

bbbales2 · 2020-07-08T04:56:50Z

@wds15 @bob-carpenter, just warning, but didn't finish or push my changes here yet, so it'll probably be confusing if you try to make sense of my responses here (because I haven't pushed code yet).

bbbales2 · 2020-07-08T14:25:21Z

@bob-carpenter is there a standard way we link from the user's guide to the functions reference?

bbbales2 · 2020-07-08T17:55:37Z

Alright I made my changes so this is ready for review again. The comments were super helpful btw.

@bob-carpenter if you want to leave the full final review to @wds15 that's fine by me, but I'd like you to take a look at the changes to the language reference and how I organized the function-reference appendix.

bob-carpenter · 2020-07-08T19:31:19Z

There's not a good way to link to the functions reference. It's doubly hard when they're getting published concurrently. You can link to a specific version or to the top-level versionless one. Obviously the versioned one will be stable and the other one less so.

bob-carpenter · 2020-07-08T19:32:07Z

I'll just approve to leave the final review for @wds15. Let me know if there's something you want to look at. This is less of a big deal than the code, because we don't introduce backward compatibility issues, etc. We can just change it in the next release if we don't like this one.

SteveBronder · 2020-07-08T20:46:23Z

Should we wait to merge this till the code goes in?

wds15 · 2020-07-11T08:19:57Z

Will read this once more on Monday.

wds15

Some minor things so far and could you please add

img/sho-ode-trajectory.png

this file is missing such that I cannot build the reference section, which makes it hard to review. Thanks.

wds15 · 2020-07-15T15:49:26Z

+In general it is better to integrate up to `t_lag` in one solve and
+then integrate from `t_lag` onwards in another. Mathematically, the
+discontinuity can make the problem ill-defined and the numerical integrator
+may behave erratically around it.


Could we mention that in case the ODE integrator is to be "run-through" the discontinuity, then it's strongly advisable to output some function values in the vicinity of the discontinuity? There are discontinuities which are not jumps, but rather sudden (but still continuous) changes - for example in forcing functions.

then it's strongly advisable to output some function values in the vicinity of the discontinuity

I don't think this will improve the behavior of the integrator around those points though. Intermediate outputs are just interpolated from the actual path the solver took.

I guess I could suggest this if there was something to do with the output. Like make plots or something. I just don't think there's an obvious way to diagnose if there are problems or not here.

No, it really does make a difference. I had experienced this myself when coding dosing events as approximate delta peaks (using normal distributions). Only when I added observation time points around the dosing time, only then the integrator picked up the dosing event. The big disadvantage of the approach you propose to restart is that you must restart the integration process with the initials being parameters whereas if you keep integrating in a single go, then - for pharmacokinetic problems at least - you can use data as the initial. This will dramatically speedup the solve process as the sensitivity system is a lot smaller as compared to making initials varying.

wds15 · 2020-07-15T15:53:57Z

-section](#stiff-ode.section).  In a nutshell, the stiff solvers are
-slower, but more robust; how much so depends on the system and the
-region of parameter space.  The function signatures for Stan's ODE
+section](#stiff-ode.section). The function signatures for Stan's ODE


In this section we talk of "stiff solvers are"... and so on. We only have a single stiff solver.

It could be worthwhile to mention that the non-stiff Adams is a suitable choice for - obviously non-stiff - problems where the ODE RHS is very expensive to calculate. This is due to the attempt of Adams to do a higher order approximation of the function in order to make large steps and in turn avoid excessive calls to the ODE RHS (whereas RK45 just calls brutally the ODE RHS very often).

This is due to the attempt of Adams to do a higher order approximation of the function in order to make large steps and in turn avoid excessive calls to the ODE RHS (whereas RK45 just calls brutally the ODE RHS very often).

I think we should work on a separate workflow ODE document because we have no real guidance on picking between the solvers. I'd like to just leave this for the future.

I find the text as is confusing as there is no guidance at all to pick between the two non-stiff ones. I would prefer to tell people: Start with RK45 in any case; if stiffness is a concern go with bdf and whenever the problem is non-stiff and the ODE RHS is expensive to evaluate, then try Adams (after having tried RK45).

This would reflect my intuition and I think is a fair summary. If you think we are not ready for this... ok.

bbbales2 · 2020-07-15T17:56:43Z

Oh whoops. Added the missing png.

…ng rk45/adams/bdf (design-doc #19)

Initial attempt at documenting new ODE interface (design-doc #19)

2ad0f04

bbbales2 mentioned this pull request Jul 4, 2020

Variadic argument lists for ODEs stan-dev/math#1641

Merged

2 of 5 tasks

SteveBronder reviewed Jul 6, 2020

View reviewed changes

Handling review comments (design-doc #19)

f145344

wds15 self-assigned this Jul 7, 2020

wds15 suggested changes Jul 7, 2020

View reviewed changes

bob-carpenter requested changes Jul 7, 2020

View reviewed changes

Responding to review comments (design-doc #19)

8a1b861

bob-carpenter approved these changes Jul 8, 2020

View reviewed changes

wds15 suggested changes Jul 15, 2020

View reviewed changes

Added missing ode trajectory png file (design-doc #19)

8934caa

bbbales2 added 2 commits July 15, 2020 14:17

Changed a bunch of 'integrators' to 'solvers' (desing-doc #19)

d1def24

Added note on generating output at discontinuities and info on choosi…

b2190e6

…ng rk45/adams/bdf (design-doc #19)

wds15 approved these changes Jul 17, 2020

View reviewed changes

bbbales2 merged commit 7ceb7df into master Jul 18, 2020

WardBrian deleted the feature/variadic-odes branch March 28, 2022 13:45

		* `ode`: function literal referring to a function specifying the
		system of differential equations with signature:

		region of parameter space. The function signatures for Stan's ODE
		solvers can be found in the reference manual section on ODE solvers.


		* solution times and number of vectors in the output,

		## Old integrate_ode_X ODE Interface {#old-functions-ode-solver}

Conversation

bbbales2 commented Jul 4, 2020

Submission Checklist

Summary

Copyright and Licensing

Uh oh!

bbbales2 commented Jul 4, 2020

Uh oh!

SteveBronder left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bbbales2 commented Jul 7, 2020

Uh oh!

wds15 commented Jul 7, 2020

Uh oh!

wds15 commented Jul 7, 2020

Uh oh!

wds15 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bob-carpenter commented Jul 7, 2020

Uh oh!

bob-carpenter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bbbales2 commented Jul 8, 2020

Uh oh!

bbbales2 commented Jul 8, 2020

Uh oh!

wds15 Jul 16, 2020 •

edited

Loading