search

mlr-org / mlr3hyperband

Successive Halving and Hyperband in the mlr3 ecosystem
https://mlr3hyperband.mlr-org.com
GNU Lesser General Public License v3.0
18 stars 5 forks source link

suggested package emoa is never loaded in code #14

Closed berndbischl closed 4 years ago

berndbischl commented 5 years ago

pls have a look how we do this for eg GenSA in mlr3tuning

SebGGruber commented 5 years ago

I don't get it - I use emoa:: at two spots in the nds_selection function. The same way as mlr3tuning uses GenSA in TunerGenSA. Is GenSA also called somewhere else? I can't find it.

berndbischl commented 5 years ago

@mllg do we HAVE to load the suggested package / import the namespace? or is this enough?

mllg commented 5 years ago

emoa should be listed in Suggests and its namespace should be loaded with requireNamespace() or mlr3misc::require_namespaces() in nds_selection(). The later is not technically required but considered to be good practice.

SebGGruber commented 5 years ago

implemented as @mllg suggested

berndbischl commented 5 years ago

actually......

a) GenSA DOES require_namespace its package. its done in the superclass Tuner. look at it. b) you could actually add the emoa package into the "packages-slot" of every tuner thats multicrit. not sure whats best. maybe lets stay as we are?

berndbischl commented 5 years ago

the cleanest solution would actually be to have a Tuner sublass for multi-crit tuning. and add the emoa package in the constructor of TunerMultiCrit to the package slot

jakob-r commented 4 years ago

at the moment we have a package slot in Tuner but we don't load packages, and we specifically have documented that packages are not automatically loaded by any method of the Tuner class.