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

Don't pass dims to the likelihood distribution #629

Merged
merged 8 commits into from
Feb 2, 2023

Conversation

tomicapretto
Copy link
Collaborator

@tomicapretto tomicapretto commented Jan 25, 2023

We are passing dims to the likelihood distribution. Internally, there's something like what follows happening

with model:
    ...
    pm.Distribution(name, **params, observed=observed, dims=dims)

However, the dims aren't used and they can bring more problems in special cases. PyMC can determine the shape of the distribution from the shape of the observed data.

For more context, I figured this out because of a collaboration with people developing a library on top of Bambi. They have a custom likelihood function where observed.ndim is 2, but Bambi was passing a single dimension. My first attempt was to add a function that added as many dimensions as needed (a4e3646) but it turns we can actually make it work without passing dimensions and just letting PyMC use the shape from observed.

@tomicapretto tomicapretto changed the title Add extra dims and coords to response when its multivariate but predictors aren't Don't pass dims to the likelihood distribution Jan 28, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jan 28, 2023

Codecov Report

Merging #629 (27bd9ea) into main (9ba92e1) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #629      +/-   ##
==========================================
- Coverage   86.63%   86.63%   -0.01%     
==========================================
  Files          37       37              
  Lines        2200     2199       -1     
==========================================
- Hits         1906     1905       -1     
  Misses        294      294              
Impacted Files Coverage Δ
bambi/backend/terms.py 97.36% <100.00%> (-0.02%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@tomicapretto tomicapretto merged commit 5ad17fd into bambinos:main Feb 2, 2023
@tomicapretto tomicapretto deleted the fake_multivariate branch February 2, 2023 12:09
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