Skip to content

fix returned k when both tails give NA#457

Open
avehtari wants to merge 2 commits into
masterfrom
fix-k-both-tails-NA
Open

fix returned k when both tails give NA#457
avehtari wants to merge 2 commits into
masterfrom
fix-k-both-tails-NA

Conversation

@avehtari

@avehtari avehtari commented Jun 26, 2026

Copy link
Copy Markdown
Member

When using pareto_smooth(, tail = "both") and both tails are such that NA is returned, the current version gives a warning

In max(left_k, right_k, na.rm = TRUE) :
  no non-missing arguments to max; returning -Inf

as max() is not able to compare NA and NA.

This PR adds a check for left_k and right_k being both NA. The new test fails without the fix and passes with the fix.

This warning came up when using brms::loo_epred() which calls E_loo() which checks both tails, and new loo v2.10.0 has switched to using posterior package for the checks

I used Claude to find the error and suggest a fix. I understand the fix. I wrote the new test.

@github-actions

Copy link
Copy Markdown

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

  • ✔️as_draws_array: 141ms -> 142ms [-0.45%, +1.02%]
  • ✔️as_draws_df: 68.6ms -> 68.5ms [-0.93%, +0.6%]
  • ✔️as_draws_list: 155ms -> 154ms [-0.77%, +0.72%]
  • ✔️as_draws_matrix: 24.5ms -> 24.5ms [-0.81%, +0.64%]
  • ✔️as_draws_rvars: 113ms -> 113ms [-1.27%, +0.14%]
  • ✔️summarise_draws_100_variables: 731ms -> 730ms [-0.32%, +0.07%]
  • 🚀summarise_draws_10_variables: 82ms -> 81.8ms [-0.56%, -0.05%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

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.

1 participant