mlrMBO

mlrMBO copied to clipboard

When we use mlr:::makeTuneControlMBO in mlr, the default uses 4*d for initial design, this should be configurable also the type of the initial design.

mbo.iters = budget - 4*length(ps$pars)
surrogate.lrn = makeLearner("regr.km", predict.type = "se", nugget = 10^-6)
mbo.ctrl = makeMBOControl()
mbo.ctrl = setMBOControlTermination(mbo.ctrl, iters = mbo.iters)
mbo.ctrl = mlr:::makeTuneControlMBO(learner = surrogate.lrn, mbo.control = mbo.ctrl)

You can pass your own design with mlr:::makeTuneControlMBO(..., mbo.design = generateDesign(n = 4*length(ps$pars), fun = randomLHS)). Is that sufficient?

Have a look at the code snippet. We are in mlr. You don't want to create the design here yourself but possibly change its size.

Wait I didn't read your full post. Maybe we should simply doc this in mlr?

But I still have the suspicion that it is better to add the init des size option back to mbo

I'm in for a better documentation. But in this case I don't favor more function arguments that kind of "rival" with existing arguments (i.e. mbo.design) and thus will clutter the code more just to take some easy work from the user. @giuseppec You might also like this setMBOControlTermination(mbo.ctrl, max.evals = budget)

it really does not make the package better to hardcode this parameter as 4 internally. expose it and close here.

(max.evals I already showed to Giuseppe)

My way of thinking: I want to try to avoid settings/arguments that conflict each other. Of course the 4 is not always perfect but it's just a default and it's super easy to change it to e.g. 5 yourself by calling mlr:::makeTuneControlMBO(..., mbo.design = generateDesign(n = 5*sum(getParamLengths(ps)), fun = randomLHS))

What is your suggestion? mlr:::makeTuneControlMBO(..., mbo.design.points.mulitplier = 5) or mlr:::makeTuneControlMBO(..., mbo.design.points = 20).

My suggestion is to expose the init.design.size in mlrMBO

And its not "super easy" currently. Giuseppe is kinda part of our internal team and he did not find this....

... because it is not well enough documented :wink:.

Maybe we have to come to a democratic decision. I am summoning @mllg, @jakobbossek @danielhorn.

I do not like the init.design.size parameter. We once had a similar parameter in mbo and decided to create designs externally. +1 for the following code by @jakob-r:

mlr:::makeTuneControlMBO(..., mbo.design = generateDesign(n = 5*sum(getParamLengths(ps)), fun = randomLHS))

i still disagree. we removed init.design.size because we wanted to simplify the interface. for mbo this is the case, now, for mlr it is not.

the point is simply: the code posted above is completely ugly. and you have to type it in for a very basic and fundamental feature option. an option that already "exists", which we have internally hardcoded with 4. that is surely not good API design.

just because we did that, does not proof it right.

its potentially minor point (if this is at least documented.....! in mlr now too! which you would not need if there would be a clearly understandable option....), but that the design here is not good seems pretty obvious to me.

Let's put it in other words: I still don't like it and I find it strage that we introduce an option in the mlr->mlrMBO Interface that does not even exist in mlrMBO. But this opinion is just so strong that I would not want to do it myself. If you find someone who does it I surely would merge it.

an option in the mlr->mlrMBO Interface that does not even exist in mlrMBO.

shall we talk about this orally? i am talking DIRECTLY about the mlrMBO API itself (yes, this helps with the mlr interaction, which was the reason to open the issue here)

I will simply do a PR

(or lets continue arguing.... but then on hangout pls)

+1, I today had a another problem / inconvenience as the init.des.size option was not there