Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fit calibrators at fit.container() #12

Merged
merged 2 commits into from
May 2, 2024
Merged

fit calibrators at fit.container() #12

merged 2 commits into from
May 2, 2024

Conversation

simonpcouch
Copy link
Contributor

@simonpcouch simonpcouch commented Apr 26, 2024

Closes #9.

Before this PR, interface was:

 library(container)
 library(modeldata)
 library(probably)
#> 
#> Attaching package: 'probably'
#> The following objects are masked from 'package:base':
#> 
#>     as.factor, as.ordered
 library(tibble)

 # create example data
 set.seed(1)
 dat <- tibble(y = rnorm(100), y_pred = y/2 + rnorm(100))

 dat
#> # A tibble: 100 × 2
#>         y y_pred
#>     <dbl>  <dbl>
#>  1 -0.626 -0.934
#>  2  0.184  0.134
#>  3 -0.836 -1.33 
#>  4  1.60   0.956
#>  5  0.330 -0.490
#>  6 -0.820  1.36 
#>  7  0.487  0.960
#>  8  0.738  1.28 
#>  9  0.576  0.672
#> 10 -0.305  1.53 
#> # ℹ 90 more rows

 # calibrate numeric predictions
 reg_cal <- cal_estimate_linear(dat, truth = y, estimate = y_pred)

 # specify calibration
 reg_ctr <-
   container(mode = "regression") %>%
   adjust_numeric_calibration(reg_cal)

 # "train" container
 reg_ctr_trained <- fit(reg_ctr, dat, outcome = y, estimate = y_pred)

 predict(reg_ctr, dat)
#> # A tibble: 100 × 2
#>         y  y_pred
#>     <dbl>   <dbl>
#>  1 -0.626 -0.257 
#>  2  0.184  0.303 
#>  3 -0.836 -0.512 
#>  4  1.60   0.479 
#>  5  0.330  0.0108
#>  6 -0.820  0.523 
#>  7  0.487  0.480 
#>  8  0.738  0.516 
#>  9  0.576  0.438 
#> 10 -0.305  0.536 
#> # ℹ 90 more rows

Created on 2024-04-26 with reprex v2.1.0

With this PR:

library(container)
library(modeldata)
library(probably)
#> 
#> Attaching package: 'probably'
#> The following objects are masked from 'package:base':
#> 
#>     as.factor, as.ordered
library(tibble)

# create example data
set.seed(1)
dat <- tibble(y = rnorm(100), y_pred = y/2 + rnorm(100))

dat
#> # A tibble: 100 × 2
#>         y y_pred
#>     <dbl>  <dbl>
#>  1 -0.626 -0.934
#>  2  0.184  0.134
#>  3 -0.836 -1.33 
#>  4  1.60   0.956
#>  5  0.330 -0.490
#>  6 -0.820  1.36 
#>  7  0.487  0.960
#>  8  0.738  1.28 
#>  9  0.576  0.672
#> 10 -0.305  1.53 
#> # ℹ 90 more rows

# specify calibration
reg_ctr <-
  container(mode = "regression") %>%
  adjust_numeric_calibration(type = "linear")

# train container
reg_ctr_trained <- fit(reg_ctr, dat, outcome = y, estimate = y_pred)

predict(reg_ctr_trained, dat)
#> # A tibble: 100 × 2
#>         y  y_pred
#>     <dbl>   <dbl>
#>  1 -0.626 -0.257 
#>  2  0.184  0.303 
#>  3 -0.836 -0.512 
#>  4  1.60   0.479 
#>  5  0.330  0.0108
#>  6 -0.820  0.523 
#>  7  0.487  0.480 
#>  8  0.738  0.516 
#>  9  0.576  0.438 
#> 10 -0.305  0.536 
#> # ℹ 90 more rows

Created on 2024-04-26 with reprex v2.1.0

Will be followed up by workflows PR updates (tidymodels/workflows#225) and a tune PR later today.

# `adjust_*()` function via `arg_match0()`.
# * `container_type`, the `type` argument either specified in `container()`
# or inferred in `fit.container()`.
check_type <- function(adjust_type,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple gaps in this logic depending on when/how different variants of the type are supplied. Will sit tight on closing those up until we address #13.

@@ -1,8 +1,11 @@
#' Re-calibrate numeric predictions
#'
#' @param x A [container()].
#' @param calibrator A pre-trained calibration method from the \pkg{probably}
#' package, such as [probably::cal_estimate_linear()].
#' @param type Character. One of `"linear"`, `"isotonic"`, or
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When putting together #13, ultimately felt that this should have a different argument than the same one supplied to container(type). I'd propose method, but will wait to Replace All until there's more feedback here.

@simonpcouch simonpcouch marked this pull request as ready for review April 30, 2024 21:43
@simonpcouch simonpcouch requested a review from topepo April 30, 2024 21:44
Comment on lines +118 to +119
arg_nm = arg,
error_call = call
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one

@simonpcouch simonpcouch merged commit 071749e into main May 2, 2024
1 check passed
@simonpcouch simonpcouch deleted the fit-calibrators branch May 2, 2024 18:17
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.

should calibration objects be pre-fitted or fitted at fit.container()?
3 participants