Add generics describing what fitted expressions compute - #435
Merged
Conversation
tidypredict_fit() returns expressions but says nothing about what they mean, and the meaning is not recoverable from their shape. Two cases where getting it wrong is silent: A LiblineaR logistic regression and a LiblineaR SVM classifier both return one expression. The first is a probability in [0, 1]; the second is an uncalibrated decision value that straddles 0. Cutting the second at 0.5 as though it were a probability misclassifies every row whose value lies between 0 and 0.5. A multiclass probability list and a quantreg::rq() fit with several tau are both named lists of expressions of the same length. In the first the values sum to one; in the second they are unrelated predictions. Add tidypredict_output_type(), tidypredict_outcome_levels() and tidypredict_normalized(), asked of the model rather than of the result. Attributes on the result were the obvious alternative and do not work: they are dropped by the subsetting, lapply() and unlist() that callers apply to a multiclass list, and tidypredict_save() serialises to YAML so they do not survive a round trip either. Only four output types exist. There is no "logit": every multiclass list goes through one shared expr_softmax(), so tidypredict_normalized() is always TRUE or NA and never FALSE. It is worth keeping so callers can rely on that rather than having to know it. Two cases are less obvious than they look. rpart, ctree, C5.0 and bagger classification return a class label and no probability at all, so they are "class" rather than "prob". xgboost's binary:hinge objective is also "class": it is wrapped in as.numeric(score >= 0) and so takes only the values 0 and 1, which makes using it as a numeric prediction wrong even though its type is numeric. There are 44 tidypredict_fit() methods but only 10 parsed-model types, so the .default methods route through parse_model() the way tidypredict_fit.default() does, and most methods sit on the pm_* classes. A backend whose parsed form does not record what is needed gets a method on the fitted class: rpart and party do not record their mode, ksvm and glm do not record their response levels once parsed, and parse_model.rqs() returns a bare list rather than a parsed model.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
T2 / phase 1b of the orbital new-backends plan.
tidypredict_fit()returns expressions but says nothing about what they mean, and the meaning is not recoverable from their shape. Two cases where guessing wrong is silent rather than loud:LiblineaRlogistic regression and aLiblineaRSVM classifier both return exactly one expression. Measured: the first lands in[0.0066, 0.971], the second straddles zero at[-2.25, 1.38]. Thresholding the second at 0.5 as though it were a probability misclassifies every row whose value falls between 0 and 0.5.quantreg::rq()fit with severaltauare both named lists of expressions of the same length. In the first, row sums are exactly 1; in the second they ran 31 to 84.The three generics
tidypredict_output_type(),tidypredict_outcome_levels(),tidypredict_normalized(). Documented at?tidypredict_metadata.Asked of the model, not attached to the result
Attributes on the fit result were the obvious design and do not work. Measured: on a named list,
x[lvl],lapply()andunlist()each drop them; on a language object adeparse/parseround trip drops them. orbital's own multiclass consumer doesclass_eqs[lvl]thenunlist(), so the metadata would be gone before it could be read — in exactly the multiclass case that motivated it.Independently,
tidypredict_save()isyaml::write_yaml()on the parsed model, so attributes do not survive a round trip either, and it cannot accept a fit result at all (it tries toparse_model()acall).Asking the model has nothing to strip, changes no existing return value, and matches the extractor-generic pattern from #433.
Only four output types, and two surprises
There is no
"logit". Every multiclass list goes through one sharedexpr_softmax(), so values always already sum to one.tidypredict_normalized()is therefore alwaysTRUEorNA, neverFALSE. Kept anyway so callers can rely on the invariant instead of knowing it, and so a future non-normalizing backend can say so rather than silently break the assumption."class"covers more than expected. rpart, ctree, C5.0 and baguette bagger classification return a class label and no probability at all — there is nothing to argmax or normalize. Andxgboost'sbinary:hingeis also"class":apply_xgb_objective()wraps it inas.numeric(score >= 0), so it takes only the values 0 and 1. That makes using it as a numeric prediction wrong even though its type is numeric. I had it as"numeric"first and changed it after checking the values.Structure
44
tidypredict_fit()methods, but only 10 parsed-model types. So the.defaultmethods route throughparse_model()the waytidypredict_fit.default()does, and most methods sit on thepm_*classes. A backend gets a fitted-class method only where the parsed form is insufficient:parse_model.rqs()returns a bare list rather than aparsed_modelparse_model()methodtypeto tell probability from decision valueWhere mode or levels had to be detected, the existing signal is reused rather than a parallel one invented —
ksvm_is_classification()is factored out ofparse_model_ksvm(), andbaggerreusesbagger_classes().Verification
Suite
FAIL 0 | WARN 6 | SKIP 1 | PASS 2675, up from 2537. The 6 warnings are pre-existing and in other files.R CMD checkwith vignettes gives 0 errors, 0 warnings (notes are a worktree.gitand the pre-existing unstatedcatboost).Every backend was checked by fitting a model and evaluating the returned expressions, not by reading source. For one representative of each output type the tests assert the values, not just the label: prob lists sum to 1, single probs stay in
[0,1], therqslist demonstrably does not sum to 1, class models return labels drawn from their levels, andbinary:hingetakes only the values 0 and 1.One trap caught this way:
kernlab::lev()returns the sorted response values for an SVR, so a naive implementation reported 25 numeric "levels" for a regression. Gated onkernlab::type(), with a test assertingNULL.Pre-existing issues noticed, not fixed here
parse_model.rqs()breaks theparse_model()contract, returning a plainlistof parsed models rather than aparsed_model. Anything routing throughparse_model()generically cannot handle it, which is whyrqsneeds fitted-class methods.parse_model_lm()callssummary()onrqfits, which runsrq.fit.br(ci = TRUE)and warns "Solution may be nonunique" once per quantile. Parsing should not need the inference machinery; it makestidypredict_fit()on anrqsnoisy.