Pareto k tail fix - Githubissues

stan-dev / posterior

The posterior R package

https://mc-stan.org/posterior/

Other

167 stars 24 forks source link

Pareto k tail fix #351

Closed n-kall closed 8 months ago

n-kall commented 8 months ago

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:

Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

codecov-commenter commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

:exclamation: 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

```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

paul-buerkner commented 8 months ago

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

github-actions[bot] commented 8 months ago

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

:ballot_box_with_check:as_draws_array: 144ms -> 144ms [-1%, +0.95%]
:exclamation::snail:as_draws_df: 74.5ms -> 75.6ms [+0.77%, +2.09%]
:ballot_box_with_check:as_draws_list: 178ms -> 178ms [-1.84%, +1.1%]
:exclamation::snail: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 commented 8 months ago

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