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

Treat offset accordingly #534

Merged
merged 9 commits into from
Jun 22, 2022
Merged

Treat offset accordingly #534

merged 9 commits into from
Jun 22, 2022

Conversation

tomicapretto
Copy link
Collaborator

This PR closes #528

See example

import arviz as az
import bambi as bmb
import numpy as np

data = bmb.load_data("carclaims")

model = bmb.Model("numclaims ~ offset(np.log(exposure))", data, family="poisson", link="log")
model
Formula: numclaims ~ offset(np.log(exposure))
Family name: Poisson
Link: log
Observations: 67856
Priors:
  Common-level effects
    Intercept ~ Normal(mu: 0, sigma: 2.5)

  Offset effects
    offset(np.log(exposure)) ~ 1
idata = model.fit(random_seed=1234)
az.summary(idata)

image

Using brms we obtain..

library(brms)

data <- read.csv("data.csv")
model <- brm(numclaims ~ offset(log(exposure)), data, family=poisson(log))
model
 Family: poisson 
  Links: mu = log 
Formula: numclaims ~ offset(log(exposure)) 
   Data: data (Number of observations: 67856) 
  Draws: 4 chains, each with iter = 2000; warmup = 1000; thin = 1;
         total post-warmup draws = 4000

Population-Level Effects: 
          Estimate Est.Error l-95% CI u-95% CI Rhat Bulk_ESS Tail_ESS
Intercept    -1.86      0.01    -1.89    -1.84 1.00     1475     1991

Draws were sampled using sampling(NUTS). For each parameter, Bulk_ESS
and Tail_ESS are effective sample size measures, and Rhat is the potential
scale reduction factor on split chains (at convergence, Rhat = 1).

@tomicapretto
Copy link
Collaborator Author

Predictions using offsets require more work on our predict method... I'll work on it in the following days.

canyon289
canyon289 previously approved these changes Jun 13, 2022
bambi/backend/pymc.py Outdated Show resolved Hide resolved
bambi/models.py Show resolved Hide resolved
bambi/models.py Outdated Show resolved Hide resolved
@canyon289 canyon289 dismissed their stale review June 13, 2022 03:25

Meant to put comment as this is still in WIP

@tomicapretto tomicapretto marked this pull request as draft June 16, 2022 01:25
@codecov-commenter
Copy link

codecov-commenter commented Jun 16, 2022

Codecov Report

Merging #534 (162d76a) into main (91903c8) will increase coverage by 0.08%.
The diff coverage is 90.00%.

❗ Current head 162d76a differs from pull request most recent head da74497. Consider uploading reports for the commit da74497 to get more accurate results

@@            Coverage Diff             @@
##             main     #534      +/-   ##
==========================================
+ Coverage   86.84%   86.93%   +0.08%     
==========================================
  Files          32       32              
  Lines        2622     2655      +33     
==========================================
+ Hits         2277     2308      +31     
- Misses        345      347       +2     
Impacted Files Coverage Δ
bambi/terms.py 76.53% <66.66%> (-1.11%) ⬇️
bambi/backend/pymc.py 81.22% <100.00%> (+0.33%) ⬆️
bambi/models.py 89.75% <100.00%> (+1.09%) ⬆️
bambi/tests/test_predict.py 100.00% <100.00%> (ø)
bambi/tests/test_built_models.py 99.00% <0.00%> (-0.04%) ⬇️
bambi/utils.py 70.96% <0.00%> (+3.22%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91903c8...da74497. Read the comment docs.

@tomicapretto tomicapretto changed the title [WIP] Treat offset accordingly Treat offset accordingly Jun 17, 2022
@tomicapretto tomicapretto marked this pull request as ready for review June 17, 2022 22:24
@tomicapretto
Copy link
Collaborator Author

@aloctavodia @canyon289 I would like approval before merging. I would like to check if you have any questions/concerns about how this works.

bambi/backend/pymc.py Outdated Show resolved Hide resolved
bambi/backend/pymc.py Outdated Show resolved Hide resolved
bambi/backend/pymc.py Outdated Show resolved Hide resolved
@@ -356,3 +354,25 @@ def test_predict_include_group_specific():

# When we include group-specific terms, these predictions are different
assert not (idata_1.posterior["y_mean"] == idata_1.posterior["y_mean"][:, :, 0]).all()


def test_predict_offset():
Copy link
Collaborator

Choose a reason for hiding this comment

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

These tests show that the model compiles and MCMC runs. What would be nice is a check that ensures the correct parameters are recovered as well. Not required for this PR just mentioning it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, and this is the case for all our tests by the way. I think there's an issue for that, but we've never worked on it.

I think it's OK to merge this as it is now. We could work on tests for correctness in the future.

tomicapretto and others added 3 commits June 22, 2022 12:13
Co-authored-by: Ravin Kumar <ravinsdrive@gmail.com>
Co-authored-by: Ravin Kumar <ravinsdrive@gmail.com>
Co-authored-by: Ravin Kumar <ravinsdrive@gmail.com>
@tomicapretto tomicapretto merged commit a69d864 into bambinos:main Jun 22, 2022
@tomicapretto tomicapretto deleted the offset branch June 22, 2022 17:58
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.

Handle offset appropriately
3 participants