Skip to content

Pareto k tail fix#351

Merged
paul-buerkner merged 4 commits into
stan-dev:masterfrom
n-kall:pareto_k_tail_fix
Mar 6, 2024
Merged

Pareto k tail fix#351
paul-buerkner merged 4 commits into
stan-dev:masterfrom
n-kall:pareto_k_tail_fix

Conversation

@n-kall
Copy link
Copy Markdown
Collaborator

@n-kall n-kall commented Mar 6, 2024

Summary

As brought up by @avehtari in issue #345, when one tail is constant, but tail = "both" is specified, it is better to return the diagnostic for the non-constant tail rather than NA.

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses:

@n-kall n-kall marked this pull request as draft March 6, 2024 10:12
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.29%. Comparing base (5403ae5) to head (8435f4a).

❗ Current head 8435f4a differs from pull request most recent head 23d3e9a. Consider uploading reports for the commit 23d3e9a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #351      +/-   ##
==========================================
- Coverage   95.31%   95.29%   -0.02%     
==========================================
  Files          50       50              
  Lines        3840     3845       +5     
==========================================
+ Hits         3660     3664       +4     
- Misses        180      181       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@n-kall n-kall marked this pull request as ready for review March 6, 2024 10:19
@paul-buerkner
Copy link
Copy Markdown
Collaborator

Looks sensible to me. Any more changes you want to do before I merge?

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 6, 2024

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

  •   :ballot_box_with_check:as_draws_array: 144ms -> 144ms [-1%, +0.95%]
  • ❗🐌as_draws_df: 74.5ms -> 75.6ms [+0.77%, +2.09%]
  •   :ballot_box_with_check:as_draws_list: 178ms -> 178ms [-1.84%, +1.1%]
  • ❗🐌as_draws_matrix: 62.4ms -> 63.7ms [+0.55%, +3.42%]
  •   :ballot_box_with_check:as_draws_rvars: 85.3ms -> 86.1ms [-0.27%, +2.18%]
  •   :ballot_box_with_check:summarise_draws_100_variables: 709ms -> 707ms [-0.63%, +0.1%]
  •   :ballot_box_with_check:summarise_draws_10_variables: 111ms -> 111ms [-0.82%, +1.43%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@n-kall
Copy link
Copy Markdown
Collaborator Author

n-kall commented Mar 6, 2024

I don't have more to add regarding this, you can merge

@paul-buerkner paul-buerkner merged commit d0deed3 into stan-dev:master Mar 6, 2024
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.

3 participants