Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stats] Migrate bayesian engine to gaussian effect model; unlock priors + CUPED #2406

Merged
merged 29 commits into from May 13, 2024

Conversation

lukesonnet
Copy link
Contributor

@lukesonnet lukesonnet commented Apr 18, 2024

THIS PR IS DESCRIBED IN THE SUMMARY BELOW AND THE FOLLOWING TWO CHILDREN WHICH HAVE BEEN MERGED IN TO IT: #2407 #2409

Summary

Removes our gaussian mean and binomial proportion bayesian models and replaces them with the gaussian effect model (already in use by quantile metrics).

This internal document covers many of the details for what and why: https://www.notion.so/growthbook/Bayesian-Model-Transition-567387e78f4444ba9718fa9d955eab1b

Documentation for this model that will be merged eventually can be found here: https://www.notion.so/growthbook/Statistical-Details-ad44401a63ea402a9aef2944541e56ba

Also tweaks risk to not always be relative when coming back from the stats engine; it now depends on the difference type.

Testing

Already confirmed differences with existing models in above notion doc, will link python notebook at a later date.

TODO:

  • Test prior rescaling for absolute differences
  • Add unit tests for posterior distributions (this is covered by the existing quantile test)

Follow-up PRs:

Copy link

github-actions bot commented Apr 18, 2024

Your preview environment pr-2406-bttf has been deployed.

Preview environment endpoints are available at:

1 similar comment
Copy link

Your preview environment pr-2406-bttf has been deployed.

Preview environment endpoints are available at:

@lukesonnet lukesonnet requested a review from jdorn April 23, 2024 16:40
packages/stats/gbstats/bayesian/tests.py Outdated Show resolved Hide resolved
packages/stats/gbstats/bayesian/tests.py Outdated Show resolved Hide resolved
packages/stats/gbstats/bayesian/tests.py Outdated Show resolved Hide resolved
packages/stats/gbstats/bayesian/tests.py Outdated Show resolved Hide resolved
packages/stats/gbstats/bayesian/tests.py Show resolved Hide resolved
packages/stats/gbstats/bayesian/tests.py Outdated Show resolved Hide resolved
packages/stats/tests/bayesian/test_tests.py Show resolved Hide resolved
packages/stats/tests/bayesian/test_tests.py Outdated Show resolved Hide resolved
packages/stats/gbstats/gbstats.py Outdated Show resolved Hide resolved
Copy link
Contributor

@lukebrawleysmith lukebrawleysmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@lukesonnet lukesonnet changed the title [stats] Migrate bayesian engine to gaussian effect model [stats] Migrate bayesian engine to gaussian effect model; unlock priors + CUPED May 10, 2024
@lukesonnet lukesonnet merged commit 1bfa804 into main May 13, 2024
5 of 6 checks passed
@lukesonnet lukesonnet deleted the ls/bayesian-revamp branch May 13, 2024 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants