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

pre-extract workflow parameter set #641

Merged
merged 2 commits into from
Mar 20, 2023
Merged

pre-extract workflow parameter set #641

merged 2 commits into from
Mar 20, 2023

Conversation

simonpcouch
Copy link
Contributor

The extraction of parameter sets from workflows is now much quicker, though it still happens for every resample. In the boilerplate mtcars bootstrap fit, it accounts for ~13% of total evaluation time with main dev. Since the workflow is not an index of the loop over tune_grid_loop_iter, we can pre-compute the parameter set and pass it as an argument.

With main dev:

library(tidymodels)

set.seed(1)

bench::mark(
  total = fit_resamples(linear_reg(), mpg ~ ., bootstraps(mtcars, 100)),
  iterations = 5
)
#> # A tibble: 1 × 6
#>   expression      min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr> <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 total         2.71s    2.73s     0.363    46.3MB     6.96

Created on 2023-03-17 with reprex v2.0.2

With this PR:

#> # A tibble: 1 × 6
#>   expression      min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr> <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 total         2.35s    2.39s     0.415    43.5MB     7.40

Created on 2023-03-17 with reprex v2.0.2

Copy link
Member

@EmilHvitfeldt EmilHvitfeldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

one question: Why did you set params = hardhat::extract_parameter_set_dials(workflow) as the argument in tune_grid_loop_iter() but params as the argument in tune_grid_loop_iter_safely()?

@simonpcouch
Copy link
Contributor Author

simonpcouch commented Mar 17, 2023

Not for good reason. :) I had thought some functions in finetune hooked into tune_grid_loop_iter() at a lower level than they do and thus would not have passed that argument, but they use the exported interface. Removing that default!

@simonpcouch simonpcouch merged commit 1ae1a2b into main Mar 20, 2023
@simonpcouch simonpcouch deleted the param-set branch March 20, 2023 14:36
@github-actions
Copy link

github-actions bot commented Apr 4, 2023

This pull request 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 Apr 4, 2023
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 this pull request may close these issues.

2 participants