implement mohamed's solution (#12#issuecomment-451416440) #13
Conversation
|
My editor removed some empty spaces automatically. But I guess it's fine. |
There was a problem hiding this comment.
These changes look good to me. Also if you manage to find a reproducible example (with a certain seed) that generates the error in Turing's Travis (or hits the max with this PR), please file an issue so I can take a closer look.
Also, are these the only logs we need to worry about?
|
@xukai92 could the same fix be applied to similar transforms, e.g. |
| @@ -276,11 +276,11 @@ function logpdf_with_trans( | |||
|
|
|||
| sum_tmp = zero(eltype(x)) | |||
| z = x[1] | |||
| lp += log(z + ϵ) + log(one(T) - z + ϵ) | |||
| lp += log(max(0, z + ϵ)) + log(max(0, one(T) - z + ϵ)) | |||
There was a problem hiding this comment.
perhaps we can overload log(x)=log(max(0, x))? This would fix all uses of log's, e.g. those in univariate distributions with lower / upper bounds.
There was a problem hiding this comment.
May be but that would give a stack overflow. _log(x) = log(max(0, x)) would do, replacing all uses of log with _log.
There was a problem hiding this comment.
Or we'd name it something like logsafe so that it's more informative?
|
Closed in favour of #14 |
Resolving #12