Add expected_residual_lifetime predictions to ShiftedBetaGeoModel
#2010
Conversation
ColtAllen
changed the title
ShiftedBetaGeometric Distribution Blockexpected_residual_lifetime predictions to ShiftedBetaGeoModel
github-actions
bot
added
the
good second issue
| samples = rng.geometric(p, size=size) | ||
|
|
||
| return samples | ||
| # prevent log(0) by clipping small p samples |
There was a problem hiding this comment.
I wouldn't do this. If the only reason is the test, relax it or seed it. I can easily make a test that still fails with this hack.
There was a problem hiding this comment.
PPCs occasionally fail during notebook testing, and it's no different when reformulating the RV with the np.ceil(np.log(U) / np.log(1 - p))) trick.
That said, most users will be doing MAP fits with this model and evaluating with the post-inference predictive methods. Ardent Bayesians may be annoyed by the lack of PPCs, but this bug isn't something that would break a typical workflow.
There was a problem hiding this comment.
I'm no longer opposed to this truncation, because numpy does an even more aggressive truncation so that it can return integer (as those can't represent infinity): https://github.com/numpy/numpy/blob/4992eaf12a8c7a3eeb0f6d13f832a05f21e3834c/numpy/random/src/distributions/distributions.c#L996-L997
I would open an issue to consider returning floats, in which case both infinity can be returned or larger values if one writes the geometric as geom = np.ceil(np.log(u) / np.log1p(-p)) without casting to integer
There was a problem hiding this comment.
In fact you can truncate as high as 1e-35 without changing the behavior, as that will still be higher than the cutoff point for any value drawn by numpy:
numpy_cutoff = 9.223372036854776e+18
largest_uniform = np.nextafter(1, 0)
beta_clip = 1e-35
assert np.log(largest_uniform) / np.log1p(-beta_clip) >= numpy_cutoffThere was a problem hiding this comment.
maybe just leave a comment in the code about this discussion?
|
Have we hit a token limit in the PR labeler? ERROR: failed to build: failed to solve: ubuntu:latest: failed to resolve source metadata for docker.io/library/ubuntu:latest: failed to authorize: failed to fetch oauth token: unexpected status from POST request to https://auth.docker.io/token: 503 Service Unavailable: <html><body><h1>503 Service Unavailable</h1> |
ColtAllen
removed
good second issue
…#2010) * fix sBG RV bug * assert var>=0 * fixed mmm transformers numpy import * ruff format * init commit for add expected_residual_lifetime * fix dim names in unit tests * fix discount rate mypy error * test_fixtures * WIP sbg erl fixtures * WIP fixtures * fixed erl expression and nb tests pass * tighten rtol and clean up nb * add TODO on float vs int in rng_fn * update doc references for sbg * update doc references from latest to stable --------- Co-authored-by: Juan Orduz <juanitorduz@gmail.com>
5 participants
Description
This began as a bug fix for prior/posterior predictive checks in
ShiftedBetaGeoModel, but I decided to expand this PR & add another predictive method while waiting for unrelated CI checks to be resolved.Related Issue
Checklist
pre-commit.ci autofixto auto-fix.📚 Documentation preview 📚: https://pymc-marketing--2010.org.readthedocs.build/en/2010/