mlr-org / ParamHelpers

Helpers for parameters in black-box optimization, tuning and machine learning.
https://paramhelpers.mlr-org.com
Other
25 stars 9 forks source link

add trafo parameter for discretevector #212

Closed rcannood closed 5 years ago

rcannood commented 5 years ago

Would it be possible to add the trafo function for the discretevectorparam?

[Edit: the proposed changes happened in commit 92da645ff10dd2a0933f2241c6e6dc454c79cfe7. The other commits are small improvements to unit tests and documentation and might cause confusion when looking at "Files changed".]

It's absolutely vital for a function in a package I wish to upload to CRAN.

I ran a R CMD check and a revdepcheck, and they both showed that this change would not break any downstream packages.

R CMD check

── R CMD check results ──────────── ParamHelpers 1.13 ────
Duration: 3m 2.6s

0 errors ✔ | 0 warnings ✔ | 0 notes ✔

R CMD check succeeded

revdepcheck

── CHECK ────────────────────────────────── 16 packages ──
✔ CEGO 2.3.0                             ── E: 0     | W: 0     | N: 0                                                                                                                                            
✔ CEGO 2.3.0                             ── E: 0     | W: 0     | N: 0                                                                                                                                            
✔ llama 0.9.2                            ── E: 0     | W: 0     | N: 0                                                                                                                                            
✔ aslib 0.1                              ── E: 1     | W: 0     | N: 0                                                                                                                                            
✔ metagen 1.0                            ── E: 1     | W: 0     | N: 0                                                                                                                                            
✔ cmaesr 1.0.3                           ── E: 0     | W: 0     | N: 0                                                                                                                                            
✔ ecr 2.1.0                              ── E: 0     | W: 0     | N: 0                                                                                                                                            
✔ flacco 1.7                             ── E: 0     | W: 0     | N: 0                                                                                                                                            
✔ OpenML 1.8                             ── E: 0     | W: 0     | N: 0                                                                                                                                            
✔ bnclassify 0.4.2                       ── E: 0     | W: 2     | N: 3                                                                                                                                            
✔ randomsearch 0.2.0                     ── E: 0     | W: 0     | N: 0                                                                                                                                            
✔ mlrCPO 0.3.4-2                         ── E: 0     | W: 0     | N: 1                                                                                                                                            
✔ SIAMCAT 1.0.0                          ── E: 0     | W: 0     | N: 0                                                                                                                                            
✔ smoof 1.5.1                            ── E: 0     | W: 0     | N: 0                                                                                                                                            
✔ tuneRanger 0.4                         ── E: 0     | W: 0     | N: 0                                                                                                                                            
✔ mlrMBO 1.1.2                           ── E: 0     | W: 0     | N: 1                                                                                                                                            
✔ mlr 2.13                               ── E: 0     | W: 0     | N: 2                                                                                                                                            
OK:                                                                                                                                                                                                             
BROKEN: 0
Total time: 49 min
coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.1%) to 92.188% when pulling b9b88461e419c6f1dabe89b2de60040c0e50725e on rcannood:master into 649d1286663fccb1a19d9cbc075d06f4e8d6de00 on berndbischl:master.

rcannood commented 5 years ago

I improved one of the unit tests a little bit, as a way of thanking you for adding the trafo parameter :)

jakob-r commented 5 years ago

I improved one of the unit tests a little bit, as a way of thanking you for adding the trafo parameter :)

Okay, I got confused, why these changes were made. Could you add a test that also uses trafos on discrete vectors?

rcannood commented 5 years ago

Sorry for the confusion :)

It makes sense that I would also add a test for checking whether the trafo works on discrete vectors; this has been added in b9b8846.

jakob-r commented 5 years ago

As far as I'm concerned, it can be merged. @berndbischl?

rcannood commented 5 years ago

:bowing_man:

jakob-r commented 5 years ago

Merged

rcannood commented 5 years ago

Thanks a lot!