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

Adding Horseshoe Prior #836

Merged
merged 19 commits into from
Sep 1, 2024
Merged

Conversation

julianlheureux
Copy link
Contributor

Hello! In this pull request, I added the horseshoe prior to the dictionary where Bambi stores specific prior distributions.

This is Carvalho's horseshoe prior, designed for the coefficients of a multiple regression model with a large number of explanatory variables, where only a few are statistically significant.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@tomicapretto
Copy link
Collaborator

@julianlheureux thanks for the contribution! This is something we wanted for a long time.

Before merging, it would be nice to add a test. I imagine the following would be OK as it covers the basic use cases:

  • A test where we use the prior with the default arguments.
  • A test where we use the prior with non-default arguments.

In both tests I would check that

  • The underlying PyMC distributions are of the expected families with the expected arguments
  • The sampler runs

I would not check for the absence of divergences, etc. because we can't guarantee they won't happen.

I know the PyMC part of the tests is not trivial as one has to interact with PyMC internals. So, if you want you can get started with the test and I can share a snippet showing how to test that.

@tomicapretto
Copy link
Collaborator

Thanks @julianlheureux!

There's something happening with the tests, but that's unrelated to the code contribution. It has to be with data loading.

@tomicapretto tomicapretto merged commit 0520a5f into bambinos:main Sep 1, 2024
1 of 4 checks passed
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.

2 participants