mlr-org / mlr3

mlr3: Machine Learning in R - next generation
https://mlr3.mlr-org.com
GNU Lesser General Public License v3.0
945 stars 85 forks source link

predict_types should be an active binding #851

Open pfistfl opened 2 years ago

pfistfl commented 2 years ago

Since I am stumbling over this for the nth time:

predict_type and predict_types are easy to confuse (and it happens to me quite a lot).

Solution:

predict_types should IMHO be immutable (this is a property of the learner). -> Encode as an AB and if the user tries to set it point her/him to predict_type in the error message

More generally: Do we need predict_type?

Sidenote Default

It's super annoying (and IMHO unncessary) to set predict_type = "prob". I always remember that when I try to use probabilistic measures AFTER having trained the model. And in 99.9% of cases it does not matter what predict type was set for the inducer and I could change it post-hoc (which afaik works as long as the learner is not e.g. in a resample result where things are way more difficult). Question: Should we not by default predict prob IF the learner can do it? Do we have any learners that can not predict prob?

mb706 commented 2 years ago

Is there a single learner that uses it during training?

Grepping a bit:

I'd conclude that for the few cases where predict_type is used in training, it could be a hyperparameter -- for xgboost and earth it already is, in a way, since predict_type is only used here to check if the hyperparameters are set up correctly. I think we could therefore just use $predict_type during prediction (or as an argument of $predict(); or both, with the latter overriding the former), throwing errors during prediction in the few cases where different HPs were needed during training.

pfistfl commented 2 years ago

I guess to move forward here we would need the opinion of @berndbischl and @mllg?