search

mlr-org / mlr3

mlr3: Machine Learning in R - next generation
https://mlr3.mlr-org.com
GNU Lesser General Public License v3.0
914 stars 84 forks source link

some more compatibility with new paradox version #991

Closed mb706 closed 4 months ago

mb706 commented 6 months ago

@mllg this fails with the new paradox version in this test: https://github.com/mlr-org/mlr3/blob/78ad4dd439bdc7ebfa2ca327eb6cb6e08c424e16/tests/testthat/test_benchmark.R#L473-L477

However, I don't get it: the param_values argument in the design is list(cp = 0.1) for the first learner, yet the test expects cp = 0.1 in the second row of the result. Why is that?

mb706 commented 6 months ago

is the row order in the benchmark result just coincidental and depends on the hash of objects? in this case, I would adapt this specific test to be row order independent.

mllg commented 5 months ago

Yes, this is a bad test; you should reorder the names (as you already did in this draft) and then not rely on the order, e.g.:

  trained = bmr$learners$learner
  ii = which(map_lgl(trained, function(x) "cp" %in% names(x$param_set$values))) # find learner with cp
  expect_count(ii)

  expect_equal(trained[[ii]]$param_set$values, list(xval = 0, minsplit = 12, cp = 0.1))
  expect_equal(trained[[-ii]]$param_set$values, list(xval = 0, minsplit = 12, minbucket = 2))
mb706 commented 4 months ago

@mllg this one can be merged