mrc-ide / odin

ᚩ A DSL for describing and solving differential equations in R
https://mrc-ide.github.io/odin
Other
102 stars 12 forks source link

Support for computing log density of distributions #297

Closed richfitz closed 1 year ago

richfitz commented 1 year ago

Merge after #296 as it contains those commits (see work plan in mrc-4182).

See https://github.com/mrc-ide/odin/compare/mrc-4324...mrc-4326 for diff of just these changes

This PR computes the log density for some distributions (not many, and not exhaustive unlike #296 as this needs to mirror dust for now and there are a couple of interface issues to nail down later). It is intended to be enough to implement some proof-of-concept models more than anything actually very useful.

Note that there's a difference in how we hit this logic vs the expectations - with the expectations we analyse an expression and then every time we hit a r* function (e.g., rpois) we replace the expression with a new one but most expressions pass through unchanged and we only alter things if they match our table of rules. Here, all compare expressions have the same type of call (compare(d) ~ poisson(lambda) etc) which means that we don't do any analysis of the expressions we just start substituting directly.

Note that this also exposes a bit of a pain that we write

x <- rpois(lambda)
compare(d) ~ poisson(x)

which uses entirely different forms for the two calls. See mrc-4182 for more on that

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (2ac75e4) 100.00% compared to head (942e2dd) 100.00%.

:exclamation: Current head 942e2dd differs from pull request most recent head 08963fa. Consider uploading reports for the commit 08963fa to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #297 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 48 47 -1 Lines 5517 5521 +4 ========================================= + Hits 5517 5521 +4 ``` | [Impacted Files](https://app.codecov.io/gh/mrc-ide/odin/pull/297?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mrc-ide) | Coverage Δ | | |---|---|---| | [R/differentiate-support.R](https://app.codecov.io/gh/mrc-ide/odin/pull/297?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mrc-ide#diff-Ui9kaWZmZXJlbnRpYXRlLXN1cHBvcnQuUg==) | `100.00% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/mrc-ide/odin/pull/297/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mrc-ide)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

richfitz commented 1 year ago

We think the code looks good! But we're a little confused about when we would want to use the deterministic version. Couldn't find any docs in dust about this either but maybe I am not looking in the right place

I think this comment really goes with https://github.com/mrc-ide/odin/pull/296 ?

The answer in any case is that this has proven a useful approximation as it runs way faster (but captures less of the dynamics). It's not yet clear exactly how the two relate to each other, and how you should move from one to another - some proper work needs doing on this at some point.

The only docs are two lines here. I probably need to work with the science side to write up some good motivating examples, and that might be easier once they finish with a deterministic-vs-stochastic paper they're working on.

r-ash commented 1 year ago

We think the code looks good! But we're a little confused about when we would want to use the deterministic version. Couldn't find any docs in dust about this either but maybe I am not looking in the right place

I think this comment really goes with #296 ?

Agh yes it does! Thanks