diff --git a/NEWS.md b/NEWS.md index d0cc8929a..c9113cd17 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # scoringutils (development version) +- Fixed `assert_forecast()` for nominal and ordinal forecasts: the error message for incomplete forecasts named the first *complete* forecast instead of the first incomplete one (and `NA` when all forecasts were incomplete), and the methods visibly returned the forecast object instead of `invisible(NULL)` as documented and as all other forecast types do (#1195). - Fixed `as_forecast_quantile()` for sample-based forecasts producing silently wrong quantiles or erroring when `probs` was not symmetric around 0.5 (e.g. `probs = 0.4` or `probs = c(0.1, 0.2)`). Quantiles are now computed at exactly the requested `probs` (deduplicated), and out-of-range `probs` produce a clear assertion error (#1196). - Fixed `interval_coverage()` erroring on quantile levels generated with `seq()` (e.g. `seq(0.05, 0.95, 0.05)`) because the required quantile levels were matched with an exact floating point comparison. Quantile levels are now rounded to 10 decimal places before matching, consistent with the rest of the package. Also fixed `wis()`, `interval_score()` and `quantile_score(weigh = FALSE)` returning `NaN` for forecasts that include the quantile levels 0 and 1 (which form a 100% prediction interval where alpha = 0). Scores are now finite when the observation falls inside the interval, restoring the identity between the WIS and the mean of the quantile scores; the unweighted scores return `Inf` when the observation falls outside a 100% prediction interval (#1202). - `get_pairwise_comparisons()`, and therefore `add_relative_skill()`, is now substantially faster and uses much less memory. Scores are pivoted once into a forecast unit by comparator matrix instead of being merged separately for every pair of comparators. Results are unchanged. Scores with more than one row per forecast unit and comparator now produce an informative error instead of silently comparing duplicated rows (#1221, thanks to @annakrystalli for the analysis and prototype). diff --git a/R/class-forecast-nominal.R b/R/class-forecast-nominal.R index 187d0bdc0..10cee19df 100644 --- a/R/class-forecast-nominal.R +++ b/R/class-forecast-nominal.R @@ -98,7 +98,7 @@ assert_forecast.forecast_nominal <- function( ), by = forecast_unit] if (!all(complete$correct)) { - first_issue <- complete[(correct), ..forecast_unit][1] + first_issue <- complete[!(correct), ..forecast_unit][1] first_issue <- lapply(first_issue, FUN = as.character) #nolint start: object_usage_linter duplicate_argument_linter issue_location <- paste(names(first_issue), "==", first_issue) @@ -111,7 +111,7 @@ assert_forecast.forecast_nominal <- function( ) #nolint end } - return(forecast[]) + return(invisible(NULL)) } diff --git a/R/class-forecast-ordinal.R b/R/class-forecast-ordinal.R index 4af5ab897..afc8252c4 100644 --- a/R/class-forecast-ordinal.R +++ b/R/class-forecast-ordinal.R @@ -104,7 +104,7 @@ assert_forecast.forecast_ordinal <- function( ), by = forecast_unit] if (!all(complete$correct)) { - first_issue <- complete[(correct), ..forecast_unit][1] + first_issue <- complete[!(correct), ..forecast_unit][1] first_issue <- lapply(first_issue, FUN = as.character) #nolint start: object_usage_linter duplicate_argument_linter issue_location <- paste(names(first_issue), "==", first_issue) @@ -117,7 +117,7 @@ assert_forecast.forecast_ordinal <- function( ) #nolint end } - return(forecast[]) + return(invisible(NULL)) } diff --git a/tests/testthat/test-class-forecast-nominal.R b/tests/testthat/test-class-forecast-nominal.R index c73259e57..74adb1996 100644 --- a/tests/testthat/test-class-forecast-nominal.R +++ b/tests/testthat/test-class-forecast-nominal.R @@ -30,6 +30,44 @@ test_that("as_forecast.forecast_nominal() breaks when rows with zero probability ) }) +test_that("assert_forecast.forecast_nominal() names the incomplete forecast in its error message", { + # modA is complete, modB is missing the "high" outcome + dat <- data.table( + model = rep(c("modA", "modB"), times = c(3, 2)), + observed = factor("high", levels = c("low", "medium", "high")), + predicted_label = factor( + c("low", "medium", "high", "low", "medium"), + levels = c("low", "medium", "high") + ), + predicted = c(0.2, 0.3, 0.5, 0.4, 0.6) + ) + expect_warning( + expect_error( + as_forecast_nominal(dat), + "modB" + ), + "Some forecasts have different numbers of rows" + ) + + # all forecasts incomplete - the first one should be named, not NA + dat_all_incomplete <- data.table( + model = c("modA", "modB"), + observed = factor("high", levels = c("low", "medium", "high")), + predicted_label = factor("low", levels = c("low", "medium", "high")), + predicted = c(1, 1) + ) + expect_error( + as_forecast_nominal(dat_all_incomplete), + "modA" + ) +}) + +test_that("assert_forecast.forecast_nominal() returns invisible(NULL)", { + fc <- as_forecast_nominal(na.omit(example_nominal)) + expect_invisible(assert_forecast(fc)) + expect_null(assert_forecast(fc)) +}) + # ============================================================================== # is_forecast_nominal() # nolint: commented_code_linter diff --git a/tests/testthat/test-class-forecast-ordinal.R b/tests/testthat/test-class-forecast-ordinal.R index 1b9b796b9..ca893d032 100644 --- a/tests/testthat/test-class-forecast-ordinal.R +++ b/tests/testthat/test-class-forecast-ordinal.R @@ -30,6 +30,51 @@ test_that("as_forecast.forecast_ordinal() breaks when rows with zero probability ) }) +test_that("assert_forecast.forecast_ordinal() names the incomplete forecast in its error message", { + # modA is complete, modB is missing the "high" outcome + dat <- data.table( + model = rep(c("modA", "modB"), times = c(3, 2)), + observed = factor( + "high", levels = c("low", "medium", "high"), ordered = TRUE + ), + predicted_label = factor( + c("low", "medium", "high", "low", "medium"), + levels = c("low", "medium", "high"), + ordered = TRUE + ), + predicted = c(0.2, 0.3, 0.5, 0.4, 0.6) + ) + expect_warning( + expect_error( + as_forecast_ordinal(dat), + "modB" + ), + "Some forecasts have different numbers of rows" + ) + + # all forecasts incomplete - the first one should be named, not NA + dat_all_incomplete <- data.table( + model = c("modA", "modB"), + observed = factor( + "high", levels = c("low", "medium", "high"), ordered = TRUE + ), + predicted_label = factor( + "low", levels = c("low", "medium", "high"), ordered = TRUE + ), + predicted = c(1, 1) + ) + expect_error( + as_forecast_ordinal(dat_all_incomplete), + "modA" + ) +}) + +test_that("assert_forecast.forecast_ordinal() returns invisible(NULL)", { + fc <- as_forecast_ordinal(na.omit(example_ordinal)) + expect_invisible(assert_forecast(fc)) + expect_null(assert_forecast(fc)) +}) + test_that("assert_forecast.forecast_ordinal() fails if factors are not ordered", { ex_faulty <- na.omit(data.table::copy(example_nominal)) expect_error(