Skip to content

as_forecast_*() renaming onto an existing column creates duplicate columns and silently wrong scores #1199

Description

@nikosbosse

Description

When an as_forecast_<type>() constructor is asked to rename a column onto a name that already exists in the data (e.g. predicted = "prob" while a stale predicted column is also present), as_forecast_generic() calls data.table::setnames() without checking for collisions. setnames() happily creates duplicate column names, producing a corrupted forecast object that passes validation and is then scored on the wrong column.

Reproduction (actual output)

library(scoringutils)
library(data.table)
dt <- data.table(
  model = "m", id = 1:2,
  observed = factor(c(0, 1)),
  predicted = c(0.9, 0.9),  # stale column
  prob = c(0.3, 0.7)        # the real predictions
)
fc <- as_forecast_binary(dt, predicted = "prob")
names(fc)
#> [1] "model"     "id"        "observed"  "predicted" "predicted"
score(fc)
#>     model    id predicted brier_score log_score
#> 1:      m     1       0.3        0.81 2.3025851
#> 2:      m     2       0.7        0.01 0.1053605
assert_forecast(fc)
#> NULL   (validation passes silently)

The Brier scores 0.81, 0.01 are (0.9 - 0)^2, (0.9 - 1)^2 — the stale column was scored — while the displayed predicted column shows 0.3, 0.7 (the real one). Silently wrong scores.

The quantile variant (stale quantile_level column plus quantile_level = "q") errors, but with a misleading "more than one forecast for the same target" duplicate-forecasts message rather than anything pointing to the name collision.

Cause

  • R/class-forecast.R:29as_forecast_generic() calls setnames(data, old = oldnames, new = newnames) with no check that the new names collide with existing columns. All eight as_forecast_<type>() constructors route through this function.
  • R/class-forecast.R:110-113assert_forecast_generic() never checks for duplicated column names, so the corrupted object passes validation (assert_forecast() returns NULL).

Note: the user's input data.table is not mutated (ensure_data.table() copies); only the returned object is corrupted.

Intended fix

Per maintainer decision:

  • In as_forecast_generic(), error via cli_abort() when a rename target already exists as a column that is not itself being renamed away — no silent overwrite, no deprecation period (every affected call currently produces a corrupted object, so there is no legitimate use to preserve). Identity renames (observed = "observed") and simultaneous swaps keep working.
  • Defense in depth: assert_forecast_generic() additionally errors when the data has duplicated column names, so corrupt objects from any other construction route fail validation instead of passing silently.

Part of the bug audit in #1189.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions