Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding parameter with trafo a bit tricky #248

Open
mb706 opened this issue Oct 6, 2019 · 3 comments
Open

Adding parameter with trafo a bit tricky #248

mb706 opened this issue Oct 6, 2019 · 3 comments

Comments

@mb706
Copy link
Contributor

mb706 commented Oct 6, 2019

Suppose a ParamSet contains parameters that have a trafo, and one wants to add a new parameter that also has a trafo. Old ParamHelpers could do this incrementally, by doing

par.set = c(par.set,
  makeParamSet(
    makeNumericParam("scale_pos_weight", lower = -10, upper = 10,
      trafo = function(x) 2^x)))

with paradox this is harder because the $trafo is overwritten, so one must write the $trafo function completely with all transformations.

@jakob-r
Copy link
Sponsor Member

jakob-r commented Oct 7, 2019

This is totally right. But how often does it occur? Can we prevent the problem of dynamically changing a ParamSet in another way?

In ParamHelpers we spent much time implementing stuff to allow these dynamic changes and realized in the end that we actually don't use it that much. Often it is much easier to do thin in another place.

Btw, I had a paradox ParamSet combine function that did exactly that but it got scraped:

paradox/R/ParamSet.R

Lines 248 to 278 in c7c39fd

combine = function(param_set) {
if (self$length == 0) {
return(param_set$clone())
} else if (param_set$length == 0) {
return(self$clone())
}
if (length(intersect(self$ids, param_set$ids)) > 0) {
stop ("Combine failed, because new param_set has at least one Param with the same id as in this ParamSet.")
}
new_restriction = self$restriction %??% param_set$restriction
if (!is.null(self$restriction) && !is.null(param_set$restriction)) {
new_restriction = substitute((a) && (b), list(a = self$restriction %??% TRUE, b = param_set$restriction %??% TRUE))
}
new_trafo = self$trafo %??% param_set$trafo
if (!is.null(self$trafo) && !is.null(param_set$trafo)) {
new_trafo = function(x, dict, tags) {
x = self$trafo(x, dict, tags)
x = param_set$trafo(x, dict, tags)
return(x)
}
}
ParamSet$new(
id = paste0(self$id, "_", param_set$id),
handle = self$handle$clone(), #FIXME: If the handle is actually used this might not be a good idea, throw error?
params = c(self$params, param_set$params),
dictionary = c(as.list(self$dictionary), as.list(param_set$dictionary)),
tags = union(self$tags, param_set$tags),
restriction = new_restriction,
trafo = new_trafo
)
},

@berndbischl
Copy link
Sponsor Member

Isn't this nearly the same issue?
#245

@berndbischl
Copy link
Sponsor Member

You can basically achieve exactly what you want Martin trough clean functional composition?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants