tidymodels / tune

Tools for tidy parameter tuning
https://tune.tidymodels.org
Other
277 stars 42 forks source link

int_pctl.tune_results fails with PSOCK #885

Closed topepo closed 6 months ago

topepo commented 6 months ago

Here's a reprex with sequential, multicore, and multisession (aka psock):

library(tidymodels)

set.seed(991)
delivery_split <- initial_validation_split(deliveries, prop = c(0.6, 0.2), 
                                           strata = time_to_delivery)
delivery_rs <- validation_set(delivery_split)

lm_res <- 
  linear_reg() %>% 
  fit_resamples(
    time_to_delivery ~ .,
    resamples = delivery_rs,
    control = control_resamples(save_pred = TRUE)
  )

res_seq <- int_pctl(lm_res)

library(doMC)
#> Loading required package: foreach
#> 
#> Attaching package: 'foreach'
#> The following objects are masked from 'package:purrr':
#> 
#>     accumulate, when
#> Loading required package: iterators
#> Loading required package: parallel
registerDoMC(cores = 5)

res_mc <- int_pctl(lm_res)

library(doParallel)
cl <- makePSOCKcluster(parallel::detectCores(logical = FALSE))
registerDoParallel(cl)

res_psoc <- int_pctl(lm_res)
#> Error in `purrr::map2()`:
#> ℹ In index: 1.
#> Caused by error in `check_tidy()`:
#> ! `statistics` should select a list column of tidy results.
#> Backtrace:
#>      ▆
#>   1. ├─rsample::int_pctl(lm_res)
#>   2. ├─tune:::int_pctl.tune_results(lm_res)
#>   3. │ ├─... %>% dplyr::arrange(.config, .metric)
#>   4. │ └─purrr::map2(...)
#>   5. │   └─purrr:::map2_("list", .x, .y, .f, ..., .progress = .progress)
#>   6. │     ├─purrr:::with_indexed_errors(...)
#>   7. │     │ └─base::withCallingHandlers(...)
#>   8. │     ├─purrr:::call_with_cleanup(...)
#>   9. │     └─tune (local) .f(.x[[i]], .y[[i]], ...)
#>  10. │       └─tune:::boostrap_metrics_by_config(...)
#>  11. │         ├─rsample::int_pctl(rs, .metrics, alpha = alpha)
#>  12. │         └─rsample:::int_pctl.bootstraps(rs, .metrics, alpha = alpha)
#>  13. │           └─rsample:::check_tidy(stats, std_col = FALSE)
#>  14. │             └─rlang::abort(stat_fmt_err)
#>  15. ├─dplyr::arrange(., .config, .metric)
#>  16. └─purrr::list_rbind(.)
#>  17.   └─purrr:::check_list_of_data_frames(x)
#>  18.     └─vctrs::vec_check_list(x, call = error_call)
#>  19.       └─vctrs::obj_check_list(x, ..., arg = arg, call = call)

Created on 2024-04-12 with reprex v2.0.2

Browsing rsample::check_tidy(), the error is that the worker results are:

<simpleError in comp_metrics(rs$splits[[i]], y_nm, metrics, event_level, metrics_info): could not find function "comp_metrics">

comp_metrics() is in tune (unexported). I've I make a temp version of tune with it exported, there is no failure:

library(tidymodels)

set.seed(991)
delivery_split <- initial_validation_split(deliveries, prop = c(0.6, 0.2), 
                                           strata = time_to_delivery)
delivery_rs <- validation_set(delivery_split)

lm_res <- 
  linear_reg() %>% 
  fit_resamples(
    time_to_delivery ~ .,
    resamples = delivery_rs,
    control = control_resamples(save_pred = TRUE)
  )

library(doParallel)
#> Loading required package: foreach
#> 
#> Attaching package: 'foreach'
#> The following objects are masked from 'package:purrr':
#> 
#>     accumulate, when
#> Loading required package: iterators
#> Loading required package: parallel
cl <- makePSOCKcluster(parallel::detectCores(logical = FALSE))
registerDoParallel(cl)

res_psoc <- int_pctl(lm_res)
#> Warning: ! tune detected a parallel backend registered with foreach but no backend
#>   registered with future.
#> ℹ Support for parallel processing with foreach was soft-deprecated in tune
#>   1.2.1.
#> ℹ See ?parallelism (`?tune::parallelism()`) to learn more.

Created on 2024-04-12 with reprex v2.0.2

simonpcouch commented 6 months ago

Looks like this issue can be reproduced with tune only, so I'll move the issue there. Related to tidymodels/tune#888 in this repo in that both are issues that will likely ultimately find their fix in tune, but seem unrelated since this is a foreach issue in int_pctl() and tidymodels/tune#885 is a future issue in the grid code paths.🤔

github-actions[bot] commented 5 months ago

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.