stan-dev / posterior

The posterior R package
https://mc-stan.org/posterior/
Other
167 stars 24 forks source link

rfun does not work with formulas #291

Closed ksvanhorn closed 1 year ago

ksvanhorn commented 1 year ago

Package version: posterior 1.3.1 R version: 4.2.2 Platform: macOS 13.4 (Ventura)

Example code:

library(posterior)
library(rlang)
f <- rfun(~.^2)
f(rvar(1:10))

After a pause of a second or two, the above gives an error message: "Error in dim(out) <- dim(draws)[summary_dimensions] : dims [product 1] do not match the length of object [4000]". Running the last line in debug mode revelas that is_rvar_arg gets set to FALSE.

If I replace the last line of the example with

f(. = rvar(1:10))

then there is no error and I get what looks like the right answer.

mjskay commented 1 year ago

I think this has to do with the fact that rlang's formula functions rely on ... and rfun() doesn't really support ... arguments that are rvars. So I think the bigger issue here might be to allow rfun() to support ... arguments.

Relatedly, I'd recommend against using the formula functions these days anyway, since it does not work everywhere... I would use the new short-form lambda notation from base R instead (e.g. \(x) x^2). Maybe we should deprecate the formula notation wherever we support it in {posterior}? I don't think we're consistent about it anyway.

Last minor point: I'm not sure if ~ . ^ 2 was just for the sake of an example, but as with sin (#290), ^ has an implementation for the rvar class, so it will always be faster to use it directly; e.g. rvar(1:10) ^ 2 or f <- function(x) x ^ 2; f(rvar(1:10))

ksvanhorn commented 1 year ago

Thanks. I wasn't familiar with the new short-form lambda notation, and I didn't know that there were rvar implementations for sin and ^. I looked at the documentation for posterior::rvar() and looked at the list of functions you get with help(package=posterior), but I overlooked the section in the rvar vignette that lists all these math functions on rvars.

If I use (x)x^2 instead of ~.^2, everything works just fine.

mjskay commented 1 year ago

If I use (x)x^2 instead of ~.^2, everything works just fine.

Great!

I looked at the documentation for posterior::rvar() and looked at the list of functions you get with help(package=posterior), but I overlooked the section in the rvar vignette that lists all these math functions on rvars.

Yeah, we probably should advertise math function support more clearly.