Skip to content

1202: Fix interval_coverage() floating point matching and NaN scores for quantile levels 0 and 1#1217

Draft
nikosbosse wants to merge 1 commit into
mainfrom
issue-1202-quantile-level-edge-cases
Draft

1202: Fix interval_coverage() floating point matching and NaN scores for quantile levels 0 and 1#1217
nikosbosse wants to merge 1 commit into
mainfrom
issue-1202-quantile-level-edge-cases

Conversation

@nikosbosse

Copy link
Copy Markdown
Collaborator

Description

This PR closes #1202.

Two related edge-case bugs in the quantile-based metrics:

  1. interval_coverage() errored on seq()-generated quantile levels. The check for the required quantile levels used an exact floating point comparison (necessary_quantiles %in% quantile_level), so e.g. interval_coverage(observed, predicted, seq(0.05, 0.95, 0.05), interval_range = 30) aborted with "the 0.35 and 0.65 quantiles are required" even though those quantiles were present (0.35 %in% seq(0.05, 0.95, 0.05) is FALSE). The fix rounds both sides to 10 decimal places before matching, consistent with the package's existing convention (e.g. in as_forecast_quantile()).

  2. wis(), interval_score() and quantile_score(weigh = FALSE) returned NaN for quantile levels 0 and 1. These levels are explicitly permitted by input validation and occur in real hub data ("min"/"max" quantiles), but they form a 100% interval with alpha = 0, and the penalty terms 2 / alpha * (bound - observed) * indicator evaluated to Inf * 0 = NaN. This broke the documented WIS = mean quantile score identity: wis(5, matrix(c(1, 3, 5, 7, 9), nrow = 1), c(0, 0.25, 0.5, 0.75, 1)) returned NaN while quantile_score() returned 0.4. The fix applies the indicator before the 2 / alpha factor and cancels 2 / alpha * alpha / 2 analytically in the weighted case; in the unweighted case, zero penalties stay zero and the score is Inf when the observation falls outside a 100% interval (mathematically correct, per maintainer decision). quantile_score(weigh = FALSE) gets the analogous fix (a zero weighted score maps to a zero unweighted score instead of 0/0). Outputs for all other interval ranges are unchanged.

Tests were written first and failed on the unfixed code:

FAILURE: 'test-metrics-quantile.R:627:3'
Expected `wis(5, predicted, quantile_level)` to equal 0.4.
  `actual`: NaN
`expected`: 0.4

FAILURE: 'test-metrics-quantile.R:746:3'
Expected `interval_coverage(observed, predicted, quantile_level, interval_range = 30)`
not to throw any conditions.
Actually got a <rlang_error> with message:
  ! To compute the interval coverage for an interval range of "30%",
    the 0.35 and 0.65 quantiles are required

After the fix, the full test suite passes (0 failed, 920 passed).

Dev note: bug identified by an LLM audit (#1189); fix and tests implemented with LLM support, directed by @nikosbosse.

Checklist

  • My PR is based on a package issue and I have explicitly linked it.
  • I have included the target issue or issues in the PR title as follows: issue-number: PR title
  • I have tested my changes locally.
  • I have added or updated unit tests where necessary.
  • I have updated the documentation if required (N/A — no roxygen or user-facing documentation changes needed; only code comments were added).
  • I have built the package locally and run rebuilt docs using roxygen2 (N/A — no roxygen comments were changed, so no man/ files needed rebuilding).
  • My code follows the established coding standards and I have run lintr::lint() on the changed files; no new lints were introduced.
  • I have added a news item linked to this PR.
  • I have reviewed CI checks for this PR and addressed them as far as I am able.

…antile levels 0 and 1 (#1202)

interval_coverage() now rounds quantile levels to 10 decimal places
before checking for the required quantiles, so seq()-generated levels
no longer trigger a spurious missing-quantile error.

interval_score() applies the indicator before the 2 / alpha factor and
cancels the alpha / 2 weight analytically, so quantile levels 0 and 1
(interval_range = 100, alpha = 0) yield finite scores instead of NaN,
restoring the WIS = mean quantile score identity. Unweighted scores
return Inf when the observation falls outside a 100% interval.
quantile_score(weigh = FALSE) receives the analogous fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.15%. Comparing base (06ed56d) to head (68aeedb).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1217   +/-   ##
=======================================
  Coverage   98.15%   98.15%           
=======================================
  Files          41       41           
  Lines        2225     2226    +1     
=======================================
+ Hits         2184     2185    +1     
  Misses         41       41           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nikosbosse

Copy link
Copy Markdown
Collaborator Author

Automated review (Claude Fable, directed by @nikosbosse):

Verification

  • Fail-before/pass-after confirmed: with R/ reverted to main (tests kept), exactly the 4 new tests fail (11 failures + 1 error) for the stated reasons — interval_coverage() aborts on seq() levels, and wis()/interval_score()/quantile_score(weigh = FALSE) return NaN for levels 0/1. On the branch everything passes.
  • Full suite on the branch: 0 failed, 920 passed (matches the PR claim).
  • Independent edge checks: weighted WIS = mean quantile score identity holds to 3e-15 over 500 random forecasts including levels 0/1 (also with the observation exactly on a bound); Inf locations agree between unweighted wis() and quantile_score(); new interval_score() is numerically identical to the old formula for ranges 0/10/50/90 (weighted and unweighted, incl. vectorised mixed ranges); NA propagation unchanged; interval_coverage() returns correct values (not merely no error) for seq() levels and still errors on genuinely missing quantiles. Brief decisions (10-dp rounding, option (a), sibling quantile_score fix, NEWS entry) are all respected; scope, PR title format, and NEWS entry are fine.

Non-blocking observations

  1. Low: tolerance mismatch between the fixed presence check (quantile levels rounded to 10 dp) and the downstream row filter (interval ranges rounded to 10 dp; quantile error is amplified x200 in the range). A quantile level off by ~1e-11 now passes the check but silently returns NA instead of the previous loud error. Practically unreachable via the standard pipeline (as_forecast_quantile() pre-rounds levels to 9 dp), so noting only.
  2. Informational: with weigh = FALSE and levels 0/1 present, wis() and quantile_score() are now both finite but differ by design (the 100% interval contributes its dispersion to wis() but 0 to the quantile scores at levels 0/1). This follows directly from the approved 0 -> 0 mapping; the documented identity is the weighted default, which the PR restores exactly. A short docs sentence could make this explicit.

Verdict: approve.

@nikosbosse
nikosbosse marked this pull request as draft July 18, 2026 13:53
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.

interval_coverage() floating-point matching error and NaN scores for quantile levels 0 and 1

1 participant