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

int_pctl.tune_results fails with PSOCK #885

Closed
topepo opened this issue Apr 12, 2024 · 2 comments · Fixed by #887
Closed

int_pctl.tune_results fails with PSOCK #885

topepo opened this issue Apr 12, 2024 · 2 comments · Fixed by #887

Comments

@topepo
Copy link
Member

topepo commented Apr 12, 2024

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
Copy link
Contributor

Looks like this issue can be reproduced with tune only, so I'll move the issue there. Related to #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 #885 is a future issue in the grid code paths.🤔

@simonpcouch simonpcouch transferred this issue from tidymodels/finetune Apr 16, 2024
Copy link

github-actions bot commented May 2, 2024

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.

@github-actions github-actions bot locked and limited conversation to collaborators May 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants