-
Notifications
You must be signed in to change notification settings - Fork 2
fit calibrators at fit.container()
#12
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,14 +49,95 @@ is_container <- function(x) { | |
} | ||
|
||
# ad-hoc checking -------------------------------------------------------------- | ||
check_container <- function(x, call = caller_env(), arg = caller_arg(x)) { | ||
check_container <- function(x, calibration_type = NULL, call = caller_env(), arg = caller_arg(x)) { | ||
if (!is_container(x)) { | ||
cli::cli_abort( | ||
cli_abort( | ||
"{.arg {arg}} should be a {.help [{.cls container}](container::container)}, \\ | ||
not {.obj_type_friendly {x}}.", | ||
call = call | ||
) | ||
} | ||
|
||
# check that the type of calibration ("numeric" or "probability") is | ||
# compatible with the container type | ||
if (!is.null(calibration_type)) { | ||
container_type <- x$type | ||
switch( | ||
container_type, | ||
regression = | ||
check_calibration_type(calibration_type, "numeric", container_type, call = call), | ||
binary = , multinomial = | ||
check_calibration_type(calibration_type, "probability", container_type, call = call) | ||
) | ||
} | ||
|
||
invisible() | ||
} | ||
|
||
check_calibration_type <- function(calibration_type, calibration_type_expected, | ||
container_type, call) { | ||
if (!identical(calibration_type, calibration_type_expected)) { | ||
cli_abort( | ||
"A {.field {container_type}} container is incompatible with the operation \\ | ||
{.fun {paste0('adjust_', calibration_type, '_calibration')}}.", | ||
call = call | ||
) | ||
} | ||
} | ||
|
||
types_regression <- c("linear", "isotonic", "isotonic_boot") | ||
types_binary <- c("logistic", "beta", "isotonic", "isotonic_boot") | ||
types_multiclass <- c("multinomial", "beta", "isotonic", "isotonic_boot") | ||
# a check function to be called when a container is being `fit()`ted. | ||
# by the time a container is fitted, we have: | ||
# * `adjust_type`, the `type` argument passed to an `adjust_*` function | ||
# * this argument has already been checked to agree with the kind of | ||
# `adjust_*()` function via `arg_match0()`. | ||
# * `container_type`, the `type` argument either specified in `container()` | ||
# or inferred in `fit.container()`. | ||
check_type <- function(adjust_type, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
container_type, | ||
arg = caller_arg(adjust_type), | ||
call = caller_env()) { | ||
# if no `adjust_type` was supplied, infer a reasonable one based on the | ||
# `container_type` | ||
if (is.null(adjust_type)) { | ||
switch( | ||
container_type, | ||
regression = return("linear"), | ||
binary = return("logistic"), | ||
multiclass = return("multinomial") | ||
) | ||
} | ||
|
||
switch( | ||
container_type, | ||
regression = arg_match0( | ||
adjust_type, | ||
types_regression, | ||
arg_nm = arg, | ||
error_call = call | ||
Comment on lines
+118
to
+119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice one |
||
), | ||
binary = arg_match0( | ||
adjust_type, | ||
types_binary, | ||
arg_nm = arg, | ||
error_call = call | ||
), | ||
multiclass = arg_match0( | ||
adjust_type, | ||
types_multiclass, | ||
arg_nm = arg, | ||
error_call = call | ||
), | ||
arg_match0( | ||
adjust_type, | ||
unique(c(types_regression, types_binary, types_multiclass)), | ||
arg_nm = arg, | ||
error_call = call | ||
) | ||
) | ||
|
||
adjust_type | ||
} | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,35 +1,36 @@ | ||
# adjustment printing | ||
|
||
Code | ||
ctr_reg %>% adjust_numeric_calibration(dummy_reg_cal) | ||
ctr_reg %>% adjust_numeric_calibration() | ||
Message | ||
|
||
-- Container ------------------------------------------------------------------- | ||
A postprocessor with 1 operation: | ||
A regression postprocessor with 1 operation: | ||
|
||
* Re-calibrate numeric predictions. | ||
|
||
# errors informatively with bad input | ||
|
||
Code | ||
adjust_numeric_calibration(ctr_reg) | ||
adjust_numeric_calibration(ctr_reg, "boop") | ||
Condition | ||
Error in `adjust_numeric_calibration()`: | ||
! `calibrator` is absent but must be supplied. | ||
! `type` must be one of "linear", "isotonic", or "isotonic_boot", not "boop". | ||
|
||
--- | ||
|
||
Code | ||
adjust_numeric_calibration(ctr_reg, "boop") | ||
container("classification", "binary") %>% adjust_numeric_calibration("linear") | ||
Condition | ||
Error in `adjust_numeric_calibration()`: | ||
! `calibrator` should be a <cal_regression> object (`?probably::cal_estimate_linear()`), not a string. | ||
! A binary container is incompatible with the operation `adjust_numeric_calibration()`. | ||
|
||
--- | ||
|
||
Code | ||
adjust_numeric_calibration(ctr_cls, dummy_cls_cal) | ||
container("regression", "regression") %>% adjust_numeric_calibration("binary") | ||
Condition | ||
Error in `adjust_numeric_calibration()`: | ||
! `calibrator` should be a <cal_regression> object (`?probably::cal_estimate_linear()`), not a <cal_binary> object. | ||
! `type` must be one of "linear", "isotonic", or "isotonic_boot", not "binary". | ||
i Did you mean "linear"? | ||
|
There was a problem hiding this comment.
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 proposemethod
, but will wait to Replace All until there's more feedback here.