cmu-delphi / cmu-delphi/epipredict

Methods for `quantile_pred()` change interpretation of missingness, add to errors with missingness

Open
#449 7 comments 0 reactions 1 assignee Claimed by @brookslogan View on GitHub
Dominant language
R
Stars
18
Forks
13
Avg merge
21d 58m
Merged PRs (30d)
1

Description

E.g., the `vec_proxy_equal()` implementation seems to be responsible for these changes in behavior demonstrated below. The errors are due to an upstream hardhat bug in `as_tibble.quantile_pred()`, but the changes in behavior are from epipredict replacing the default `vec_proxy_equal()` behavior. This is breaking ability to store forecasts in archives in epiprocess; see https://github.com/cmu-delphi/epiprocess/issues/631.

It's rude/frustrating for a package to change existing behavior of a class not introduced by the package itself. We'll probably need to do something like one of the following:
- Remove `vec_proxy_equal.quantile_pred`
- Tweak `vec_proxy_equal.quantile_pred` to [maybe] not change missingness treatment and not raise an error.
- If it has only fixes / perf improvements relative to hardhat, then we should contribute upstream to hardhat.
- If it has other types of changes, we should define a subclass and not change `quantile_pred` behavior.

Similarly, for other `quantile_pred` method implementations that we define, we should consider contributing methods upstream if they change existing behavior, or else use a subclass and implement our behavior-change methods there. It might also be good to contribute behavior-adding-but-not-changing methods upstream.

``` r
suppressPackageStartupMessages({
library(dplyr)
library(hardhat)
library(vctrs)
})

preds1 <- quantile_pred(matrix(c(1:3, NA, NA, NA), 2, 3, byrow = TRUE), 1:3/4)
preds2 <- vec_c(preds1, NA)

vec_detect_missing(preds1)
#> [1] FALSE FALSE
preds1 == preds1
#> [1] TRUE TRUE

vec_detect_missing(preds2)
#> [1] FALSE FALSE TRUE
preds2 == preds2
#> [1] TRUE TRUE NA
vec_equal(preds2, preds2, na_equal = TRUE)
#> [1] TRUE TRUE TRUE

suppressPackageStartupMessages({
library(epipredict)
})

vec_detect_missing(preds1)
#> [1] FALSE TRUE
preds1 == preds1
#> [1] TRUE NA

vec_detect_missing(preds2)
#> Error in `vec_slice()`:
#> ! Column `.quantile_levels` (size 9) must match the data frame (size 6).
#> ℹ In file 'slice.c' at line 188.
#> ℹ This is an internal error that was detected in the vctrs package.
#> Please report it at with a reprex () and the full backtrace.
preds2 == preds2
#> Error in `vec_slice()`:
#> ! Column `.quantile_levels` (size 9) must match the data frame (size 6).
#> ℹ In file 'slice.c' at line 188.
#> ℹ This is an internal error that was detected in the vctrs package.
#> Please report it at with a reprex () and the full backtrace.
vec_equal(preds2, preds2, na_equal = TRUE)
#> Error in `vec_slice()`:
#> ! Column `.quantile_levels` (size 9) must match the data frame (size 6).
#> ℹ In file 'slice.c' at line 188.
#> ℹ This is an internal error that was detected in the vctrs package.
#> Please report it at with a reprex () and the full backtrace.
```

Created on 2025-03-19 with [reprex v2.1.1](https://reprex.tidyverse.org)

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.