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

disappearing dependencies when cloning #356

Open
be-marc opened this issue Dec 13, 2021 · 3 comments
Open

disappearing dependencies when cloning #356

be-marc opened this issue Dec 13, 2021 · 3 comments

Comments

@be-marc
Copy link
Member

be-marc commented Dec 13, 2021

When a graph learner is deep cloned, the dependencies in the parameter set disappear in the original and cloned learner.

library("mlr3")
library("mlr3pipelines")
library("paradox")

graph_learner = GraphLearner$new( lrn("classif.rpart"))
graph_learner$param_set$add_dep("classif.rpart.cp", "classif.rpart.keep_model", CondEqual$new(TRUE))

graph_learner$param_set$deps
# >                 id                       on           cond
# > 1: classif.rpart.cp classif.rpart.keep_model <CondEqual[9]>

graph_learner_2 = graph_learner$clone(deep = TRUE)

graph_learner_2$param_set$deps                                                                                      
# > Empty data.table (0 rows and 3 cols): id,on,cond

graph_learner$param_set$deps                                                                                         
# > Empty data.table (0 rows and 3 cols): id,on,cond

Normal learners are not affected.

library("mlr3")
library("mlr3pipelines")
library("paradox")

learner = lrn("classif.rpart")
learner$param_set$add_dep("cp", "keep_model", CondEqual$new(TRUE))
learner$param_set$deps

# >    id         on           cond
#> 1: cp keep_model <CondEqual[9]>

learner_2 = learner$clone(deep = TRUE)

learner_2$param_set$deps

# >    id         on           cond
#> 1: cp keep_model <CondEqual[9]>
@pfistfl pfistfl transferred this issue from mlr-org/mlr3pipelines Dec 16, 2021
@pfistfl
Copy link
Member

pfistfl commented Dec 16, 2021

The reason seems to be the deep_clone() of the ParamSetCollection.
Marc and I debugged a similar thing in mlr3keras there the culprit seemed to be a fail in cloning ParamSetCollections.

A reproducible example might perhaps be resampling any learner that has a ParamSetCollection instead of a ParamSet, I think the problem is deep cloning of .values.

@be-marc
Copy link
Member Author

be-marc commented Dec 16, 2021

@pfistfl We might have violated the rules of ParamSetCollections?

#' A collection of multiple [ParamSet] objects.
#' * The collection is basically a light-weight wrapper / container around references to multiple sets.
#' * In order to ensure unique param names, every param in the collection is referred to with
#'   "<set_id>.<param_id>". Parameters from ParamSets with empty (i.e. `""`) `$set_id` are referenced
#'   directly. Multiple ParamSets with `$set_id` `""` can be combined, but their parameter names
#'   must be unique.
#' * Operation `subset` is currently not allowed.
#' * Operation `add` currently only works when adding complete sets not single params.
#' * When you either ask for 'values' or set them, the operation is delegated to the individual,
#'   contained param set references. The collection itself does not maintain a `values` state.
#'   This also implies that if you directly change `values` in one of the referenced sets,
#'   this change is reflected in the collection.
#' * Dependencies: It is possible to currently handle dependencies
#'      * regarding parameters inside of the same set - in this case simply
#'        add the dependency to the set, best before adding the set to the collection
#'      * across sets, where a param from one set depends on the state
#'        of a param from another set - in this case add call `add_dep` on the collection.
#'
#'   If you call `deps` on the collection, you are returned a complete table of dependencies, from sets and across sets.

I should have added the dependency to the set before creating the graph learner.

@pfistfl
Copy link
Member

pfistfl commented Dec 17, 2021

I think in may case I do not have deps and the culprit seems to be the deep cloning

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

2 participants