Fix #201: focussearch now handles discrete vector parameters #202
Conversation
also general focussearch bugfixes
| levels(newdesign[[dfindex]]) = mapping[levels(newdesign[[dfindex]])] | ||
| } | ||
| } | ||
| y = infill.crit(newdesign, models, control, par.set, design, iter, ...) |
There was a problem hiding this comment.
This is potentially slow and doesn't make use of our helpers. have a look at
par.set.disc.logic.v = filterParams(par.set, type = c("discretevector", "logicalvector"))
dfindex = getParamIds(par.set.disc.logic.v, with.nr = TRUE, repeated = TRUE)Parameters that have discrete vector params in their requirement can not be handled by infillOptFocus, so trying to do that now gives an error.
|
Incorporated your suggestions. I also found out that this cannot work when there is a parameter that has a discrete vector parameter in its requirement. I think for this to work one needs to either rewrite the requirements or change something in ParamHelpers. |
|
As @berndbischl is the inventor of foucssearch he should have a final look on it. If I see somtehing in the code I will commit. |
| @@ -8,11 +8,35 @@ | |||
| # See infillOptCMAES.R for interface explanation. | |||
| infillOptFocus = function(infill.crit, models, control, par.set, opt.path, design, iter, ...) { | |||
| global.y = Inf | |||
|
|
|||
| allRequirements = extractSubList(par.set$pars, "requires", simplify = FALSE) | |||
| allUsedVars = unique(do.call(base::c, sapply(allRequirements, all.vars))) | |||
There was a problem hiding this comment.
What is all.vars in this context? I feel stupid, but I cannot find it.
I also find it strange to use do.call(base::c, ...). Won't unlist do the trick?
There was a problem hiding this comment.
base::all.vars returning a vector of all variables used in the expression? Unfortunately it is not perfect (ideally I would want a list of all free variables) but it is the best R is giving me as it seems.
|
Incorporated your suggestions (hadn't thought of |
|
I don't think it is the fault of my commit diff that the check fails. |
|
I think we should pull this soon, or? @berndbischl |
|
can somebody pls fix the naming issues? the styleguide is really followed. for this, please put this in a branch, we cannot do this in a fork. i will proofread later then, please ping me. and check 1000 million times if all reported problems from the issue have been converted to unit tests |
|
Continue in #257 |
Also general focus search bugfixes.