Skip to content

fix returned k when both tails give NA#457

Merged
avehtari merged 2 commits into
masterfrom
fix-k-both-tails-NA
Jul 15, 2026
Merged

fix returned k when both tails give NA#457
avehtari merged 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.

@avehtari
avehtari requested a review from Copilot July 14, 2026 18:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

EDIT: Aki accidentally clicked Copilot review when trying to request review from real humans

@florence-bockting florence-bockting left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me.

@paul-buerkner

paul-buerkner commented Jul 15, 2026 via email

Copy link
Copy Markdown
Collaborator

@avehtari
avehtari merged commit d6275ec into master Jul 15, 2026
10 of 12 checks passed
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.

4 participants