mrc-4324: support for taking expectations of stochastic functions #296

richfitz · 2023-06-13T14:03:48Z

Required for differentiating models that contain stochastic distributions. Normally we cope with this in dust by putting distributions into "deterministic mode" which applies the same logic. Here, we do the transformation of a call to a stochastic function (e.g., rnorm(mu, sd)) with its expectation (mu) symbolically so that we can later differentiate the resulting expressions.

No visible change from any of this to the user, this is just support code to make the eventual autodiff support manageable

codecov · 2023-06-13T14:25:05Z

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (e069323) 100.00% compared to head (2ef5f87) 100.00%.

❗ Current head 2ef5f87 differs from pull request most recent head 791103d. Consider uploading reports for the commit 791103d to get more accurate results

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #296   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           46        47    +1     
  Lines         5502      5509    +7     
=========================================
+ Hits          5502      5509    +7

Impacted Files	Coverage Δ
R/differentiate-support.R	`100.00% <100.00%> (ø)`

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

M-Kusumgar

Nicely done, was fun to just look through the list of distributions and either try and remember their mean or google it 🙂

M-Kusumgar · 2023-06-20T01:10:09Z

R/differentiate-support.R

+  rgeom = function(expr) {
+    substitute((1 - p) / p, list(p = expr[[2]]))
+  },


I might be being a bit dumb, rgeom takes the probability of success right, so should this not be 1/p?

i realise you're making these functions yourself so could tell people that you need to put the probability of failure in the rgeom function but would be nice to have consistent behaviour with the rgeom from R function

This is consistent with rgeom (see tests); the actual random variable here is the number of observed failures before a success is observed:

x, q: vector of quantiles representing the number of failures in a sequence of Bernoulli trials before success occurs.

so in the edge case of pr = 1, we have 0 / 1 = 0 and rgeom(n, 1) is 0 (repeated n times).

Or am I misunderstanding?

ahhh apologies, I got confused with what the R function took, I thought it took a random variable, X, which was the number of Bernoulli trials to get one success, just tested it out numerically too! rgeom is definitely consistent with your expectation!

M-Kusumgar · 2023-06-20T11:16:51Z

R/differentiate-support.R

+  rwilcox = function(expr) {
+    substitute(m * n / 2, list(m = expr[[2]], n = expr[[3]]))
+  },


For anyone interested, result is here in the "Normal approximation and tie correction" section

M-Kusumgar · 2023-06-20T11:17:41Z

R/differentiate-support.R

+  rsignrank = function(expr) {
+    substitute(n * (n + 1) / 4, list(n = expr[[2]]))
+  })


Again for those interested, result is here in the "Computing the null distribution" section

M-Kusumgar · 2023-06-20T11:28:57Z

tests/testthat/helper-differentiate.R

+
+## Discrete distrbutions are somewhat harder
+expectation_discrete <- function(fd, fq, pars, tol = 1e-12) {
+  end <- do.call(fq, c(list(p = 1 - tol), unname(pars)))


again maybe this is very standard but think this could use a tiny comment to explain fq which i believe is the quantile function which will find an end value such that our expectation is an average over 1 - tol area under the graph, probably a way to word it better than that!

Agreed with this, and maybe a link to the definitions. To make this really obvious if we come back to this in the future.

r-ash

This looks good, one comment about the TODO

r-ash · 2023-06-20T11:29:38Z

R/differentiate-support.R

+    substitute(1 / rate, list(rate = expr[[2]]))
+  },
+  rf = function(expr) {
+    ## TODO: only valid for df2 > 2!


Do you want to add some asserts to check these edge cases?

r-ash · 2023-06-20T13:48:17Z

tests/testthat/helper-differentiate.R

+
+## Discrete distrbutions are somewhat harder
+expectation_discrete <- function(fd, fq, pars, tol = 1e-12) {
+  end <- do.call(fq, c(list(p = 1 - tol), unname(pars)))


Agreed with this, and maybe a link to the definitions. To make this really obvious if we come back to this in the future.

weshinsley

Ok - with discussion with the team, I think I understand this one, and looks good - version bump when we get to merging it

weshinsley · 2023-06-21T15:29:05Z

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: odin
 Title: ODE Generation and Integration
-Version: 1.5.0
+Version: 1.5.1


Bump after PR #295 is merged

weshinsley · 2023-06-21T15:30:30Z

tests/testthat/test-run-general.R

@@ -813,7 +813,7 @@ test_that_odin("overlapping graph", {
    list(y * p, p + p2)
  }
  cmp <- deSolve::ode(1, tt, f, NULL)
-  tol <- variable_tolerance(mod, js = 1e-6)
+  tol <- variable_tolerance(mod, js = 3e-6)


This will go away when #295 is merged

richfitz marked this pull request as ready for review June 13, 2023 17:50

richfitz requested review from weshinsley and r-ash June 13, 2023 17:50

richfitz mentioned this pull request Jun 14, 2023

Support for computing log density of distributions #297

Merged

richfitz and others added 6 commits June 15, 2023 15:13

Add support for taking expectations of expressions

d8871be

Expand tests, with fixes

6471ab0

Expand testing

5b32a37

Bump version

d107ba7

Fix lint

07d266f

Tweak tolerance on test

71648d6

richfitz force-pushed the mrc-4324 branch from 5c71062 to 71648d6 Compare June 15, 2023 14:13

M-Kusumgar reviewed Jun 20, 2023

View reviewed changes

r-ash approved these changes Jun 20, 2023

View reviewed changes

weshinsley approved these changes Jun 21, 2023

View reviewed changes

Add explanation

791103d

richfitz merged commit 1653828 into master Jun 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mrc-4324: support for taking expectations of stochastic functions #296

mrc-4324: support for taking expectations of stochastic functions #296

richfitz commented Jun 13, 2023 •

edited

Loading

codecov bot commented Jun 13, 2023 •

edited

Loading

M-Kusumgar left a comment

M-Kusumgar Jun 20, 2023

M-Kusumgar Jun 20, 2023

richfitz Jun 26, 2023

M-Kusumgar Jun 26, 2023

M-Kusumgar Jun 20, 2023

M-Kusumgar Jun 20, 2023

M-Kusumgar Jun 20, 2023

r-ash Jun 20, 2023

r-ash left a comment •

edited

Loading

r-ash Jun 20, 2023

r-ash Jun 20, 2023

weshinsley left a comment

weshinsley Jun 21, 2023

weshinsley Jun 21, 2023

mrc-4324: support for taking expectations of stochastic functions #296

mrc-4324: support for taking expectations of stochastic functions #296

Conversation

richfitz commented Jun 13, 2023 • edited Loading

codecov bot commented Jun 13, 2023 • edited Loading

Codecov Report

M-Kusumgar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

r-ash left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weshinsley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richfitz commented Jun 13, 2023 •

edited

Loading

codecov bot commented Jun 13, 2023 •

edited

Loading

r-ash left a comment •

edited

Loading