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 defer negative binomial to Poisson at low levels of overdispersion #432

Merged
merged 7 commits into from
Jul 22, 2023

Conversation

sbfnk
Copy link
Contributor

@sbfnk sbfnk commented Jul 20, 2023

Existing comment in the stan code refers to avoiding overflows so will have to monitor that.

This also reinstates the tests for parameter recovery with the secondary model and fixes a bug where observation weights weren't used in the negative binomial model.

Closes #431
Addresses part of #429
Fixes #349

Copy link
Contributor

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

LGTM. Version and news please otherwise good to go - depending on CI

@@ -85,11 +79,11 @@ vector report_log_lik(int[] cases, vector reports,
real sqrt_phi = 1e5;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can also drop this.

@github-actions
Copy link
Contributor

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 0fbf93e is merged into main:

  •   :ballot_box_with_check:default: 1.16m -> 1.12m [-18.84%, +12.4%]
  •   :ballot_box_with_check:no_delays: 1.12m -> 1.13m [-22.98%, +25.12%]
  •   :ballot_box_with_check:random_walk: 20.6s -> 21.6s [-15.44%, +24.87%]
  •   :ballot_box_with_check:stationary: 39.7s -> 39.4s [-11.35%, +9.97%]
  • ❗🐌uncertain: 1.46m -> 1.66m [+0.88%, +26.06%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@github-actions
Copy link
Contributor

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 2833027 is merged into main:

  •   :ballot_box_with_check:default: 49.7s -> 48.4s [-15.99%, +10.94%]
  •   :ballot_box_with_check:no_delays: 49.4s -> 51.9s [-5.71%, +16.02%]
  •   :ballot_box_with_check:random_walk: 15.4s -> 14.4s [-16.22%, +3.58%]
  •   :ballot_box_with_check:stationary: 29.1s -> 29.8s [-9.16%, +13.88%]
  •   :ballot_box_with_check:uncertain: 1.14m -> 1.07m [-17.1%, +5.44%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@github-actions
Copy link
Contributor

This is how benchmark results would change (along with a 95% confidence interval in relative change) if aad2527 is merged into main:

  •   :ballot_box_with_check:default: 53.3s -> 54s [-13.3%, +15.95%]
  •   :ballot_box_with_check:no_delays: 55s -> 54.2s [-10.54%, +7.66%]
  •   :ballot_box_with_check:random_walk: 15.7s -> 16s [-4.12%, +8.15%]
  •   :ballot_box_with_check:stationary: 34.2s -> 33.1s [-16.13%, +9.36%]
  •   :ballot_box_with_check:uncertain: 1.22m -> 1.25m [-12.44%, +17.96%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@sbfnk
Copy link
Contributor Author

sbfnk commented Jul 22, 2023

Two checks failing for apparently unrelated reasons.

@sbfnk sbfnk merged commit 1ad239f into main Jul 22, 2023
9 of 11 checks passed
@sbfnk sbfnk deleted the fix-poisson branch July 22, 2023 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants