search

tidymodels / dials

Tools for creating tuning parameter values
https://dials.tidymodels.org/
Other
111 stars 26 forks source link

move `mtry_prop` from rules to dials #233

Closed simonpcouch closed 2 years ago

simonpcouch commented 2 years ago

So that bonsai need not take on a rules dependency for this parameter!

Bumping version so we can require a specific dev version of this package in rules. Will be accompanied by a PR to rules shortly. :)

simonpcouch commented 2 years ago

Update: I don't think bonsai actually needs to make use of the mtry_prop parameter. Leaving this and tidymodels/rules#54 open in case it's otherwise helpful to make this switch, but feel free to close if not!

hfrick commented 2 years ago

have we settled on the API where the default is mtry is a count but you can switch to a proportion with counts = FALSE in set_engine()? (as you did for lightgbm, and already has been done for xgboost)

If so, would it make sense to switch rules over to that API as well? It would be nice to have a consistent approach if it's the same problem. cc @topepo

simonpcouch commented 2 years ago

Thanks for the review. :) Added a docs section clarifying our convention there.

github-actions[bot] commented 2 years ago

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.