allow threshold tuning in tuneWrapper

[berndbischl]

Either have option or add the old wrapper for that.

option might be simpler.

[jakob-r]

an option for tune.threshold within the tuneWrapper makes it a bit nasty IMHO i just noticed. Maybe bring back a wrapper. You can always wrap wrappers. So it should be okay, no?

[jakob-r]

What about this kind of implementation? (Yes - it is an option and no seperate wrapper) cd4e140110ca1231ead45f75e9ed4687de76f973

[berndbischl]

I needed to revert this. sorry. I have to release 2.2 today! You code is not lost but in history.

We need to discuss this. One thing that came to my mind: Should this thresh-tuning not also be done during normal calls to tuneParams?

[berndbischl]

We agreed today that it might be good to a) have an option for this in OptControl b) do the tuneThreshold in evalOptState c) bonus: store the threshold in opt path

[jakob-r]

a) You mean i should put this option in each version of makeTuneControlGrid, makeTuneControlMBO, ... ? Like makeTuneControlGrid(..., also.tune.threshold = TRUE)

[jakob-r]

should be finished here e60893b92a4d558282095c1e25cf8a586c57f397 please reread

[berndbischl]

Review:

1) assertFlag exists 2) We wanted to have this for FeatSel, too. We should do as many arg checks in the base class as possible.

3)

      if (!learner2$predict.type == "prob")
        stop("'tune.threshold = TRUE' requires a learner with the predict.type prob")
      tune.th.r = tuneThreshold(r$pred, getFirst(measures))

This check is bad on many levels: a) The check should be done in tuneThreshold. Maybe it is already there. IF you want an extra GOOD check you should do it before computation starts. Not in the middle of it.

[berndbischl]

I am currently on it.

[berndbischl]

Regearding 3): The check is on setThreshold, which seems OK.

[berndbischl]

3) I added that check to checkTuneParset. This is where it should be.....

[berndbischl]

4) Please make sure the option is added for all FEatSelControls

5) We need unit tests for tuning and featsel. Both with multi- and binary class, because both are different

[berndbischl]

6) Make sure you read what I did.

Now take over.

[jakob-r]

more is done here 675f3343105c5a5a2e50771b477af316a8cbac69

[jakob-r]

please review again

[berndbischl]

I am reviewing this now, 2 notes for later:

1) The tuning (in the unit test) seems a bit slow.

2) Do we need to support this for multi criteria tuning as well? How / can we handle this there?

[berndbischl]

3) We also do not allow to set further CMAES params in the threshold tuning.

[berndbischl]

done