Description
The docs for the sample-based metrics state that when observed is a single number, predicted can be given as a vector of samples (see ?ae_median_sample: "if n = 1, predicted can just be a vector"), and assert_input_sample() (R/metrics-sample.R:17-22) explicitly accepts this shape. However, four of the sample metrics either silently return wrong results or crash on this documented input:
devtools::load_all()
ae_median_sample(1, c(0, 2, 4))
#> [1] 1 1 3 # wrong; should be |1 - median(c(0, 2, 4))| = 1
se_mean_sample(1, c(0, 2, 4))
#> [1] 1 1 9 # wrong; should be (1 - mean(c(0, 2, 4)))^2 = 1
bias_sample(1, c(0, 2, 4))
#> Error in rowSums(predicted <= observed) :
#> 'x' must be an array of at least two dimensions
mad_sample(predicted = c(0, 2, 4))
#> Error in rep(NA_real_, nrow(predicted)) : invalid 'times' argument
The equivalent 1xN matrix input gives the correct answers everywhere, and crps_sample() / dss_sample() / pit_histogram_sample() already handle the vector form correctly via inline vector-to-matrix conversions.
Cause
ae_median_sample() (R/metrics-sample.R:145) and se_mean_sample() (R/metrics-sample.R:174) call as.matrix(predicted), which turns a length-N vector into an Nx1 matrix, i.e. N one-sample forecasts; the scalar observed is then recycled, producing a wrong length-N result.
bias_sample() calls ncol(predicted) (R/metrics-sample.R:95) and rowSums() (R/metrics-sample.R:101/108) on a dimensionless vector and errors.
mad_sample() calls nrow(predicted) on a vector (R/metrics-sample.R:414), which is NULL, so rep(NA_real_, NULL) errors.
Additionally: bias_sample() docstring formula is wrong
The roxygen deqn for the integer case (R/metrics-sample.R:54) reads B_t = 1 - (P_t(x_t) + P_t(x_t + 1)), but the code (correctly) computes 1 - (P(x_t) + P(x_t - 1)) with P the empirical CDF. For a perfect integer forecast (all samples equal to the observation) the code gives 0 while the documented formula would give -1. The docs should read P_t(x_t - 1).
Intended fix (per maintainer decision)
Treat scalar observed + vector predicted as one forecast with N samples, per the docs: add an internal helper that converts a dimensionless predicted to a 1xN matrix and use it in bias_sample(), ae_median_sample(), se_mean_sample() and mad_sample() (also deduplicating the existing inline conversions in crps_sample() and pit_histogram_sample()). Correct the bias_sample() docstring formula to P_t(x_t - 1) and rebuild docs with roxygen2.
Part of the bug audit in #1189.
Description
The docs for the sample-based metrics state that when
observedis a single number,predictedcan be given as a vector of samples (see?ae_median_sample: "if n = 1,predictedcan just be a vector"), andassert_input_sample()(R/metrics-sample.R:17-22) explicitly accepts this shape. However, four of the sample metrics either silently return wrong results or crash on this documented input:The equivalent 1xN matrix input gives the correct answers everywhere, and
crps_sample()/dss_sample()/pit_histogram_sample()already handle the vector form correctly via inline vector-to-matrix conversions.Cause
ae_median_sample()(R/metrics-sample.R:145) andse_mean_sample()(R/metrics-sample.R:174) callas.matrix(predicted), which turns a length-N vector into an Nx1 matrix, i.e. N one-sample forecasts; the scalarobservedis then recycled, producing a wrong length-N result.bias_sample()callsncol(predicted)(R/metrics-sample.R:95) androwSums()(R/metrics-sample.R:101/108) on a dimensionless vector and errors.mad_sample()callsnrow(predicted)on a vector (R/metrics-sample.R:414), which isNULL, sorep(NA_real_, NULL)errors.Additionally:
bias_sample()docstring formula is wrongThe roxygen deqn for the integer case (R/metrics-sample.R:54) reads
B_t = 1 - (P_t(x_t) + P_t(x_t + 1)), but the code (correctly) computes1 - (P(x_t) + P(x_t - 1))withPthe empirical CDF. For a perfect integer forecast (all samples equal to the observation) the code gives 0 while the documented formula would give -1. The docs should readP_t(x_t - 1).Intended fix (per maintainer decision)
Treat scalar
observed+ vectorpredictedas one forecast with N samples, per the docs: add an internal helper that converts a dimensionlesspredictedto a 1xN matrix and use it inbias_sample(),ae_median_sample(),se_mean_sample()andmad_sample()(also deduplicating the existing inline conversions incrps_sample()andpit_histogram_sample()). Correct thebias_sample()docstring formula toP_t(x_t - 1)and rebuild docs with roxygen2.Part of the bug audit in #1189.