add prompt for params that will not be tuned (#549)

* implement warning for params that will not be tuned, update existing snaps

* add tests for warning re: params that will not tune

* add NEWS bullet

* clarify dependencies

* Apply suggestions from code review

Co-authored-by: Davis Vaughan <davis@rstudio.com>

* `expect_silent` -> `expect_warning` with `regexp = NA`

slightly more granular, but still not as exact as testing with dev `expect_no_warning` with a class

* simplify conditions, rephrase prompt

only check the model object, as there's not a well-defined analogue for the "engine" in recipes. this also helps simplify the cli machinery.

* restore current warning, elevate new warning to error

* extend testing

* update NEWS

* remove outdated remote

* move new check into sub-function

* spelling issue

* check installs for workflow (not just the model object)

* Revert "check installs for workflow (not just the model object)"

This reverts commit 8338639.

Co-authored-by: Max Kuhn <mxkuhn@gmail.com>
Co-authored-by: Davis Vaughan <davis@rstudio.com>

DESCRIPTION

@@ -27,7 +27,7 @@ Imports:
     GPfit,
     hardhat (>= 1.2.0),
     lifecycle (>= 1.0.0),
-    parsnip (>= 1.0.1.9000),
+    parsnip (>= 1.0.2),
     purrr (>= 0.3.2),
     recipes (>= 1.0.0),
     rlang (>= 1.0.2),
@@ -55,5 +55,4 @@ Language: en-US
 LazyData: true
 Roxygen: list(markdown = TRUE)
 RoxygenNote: 7.2.1.9000
-Remotes: 
-    tidymodels/parsnip
+

NEWS.md

@@ -4,7 +4,10 @@
 
 * The `collect_metrics()` method for racing objects was removed (and is now in the finetune package).
 
-* The `control_bayes()` got a new argument `verbose_iter` that is used to control the verbosity of the bayesian calculations. This change means that the `verbose` argument is being passed to `tune_grid()` to control its verbosity.
+* Improved prompts related to parameter tuning. When tuning parameters are supplied that are not compatible with the given engine, `tune_*()` functions will now error. (#549)
+
+* The `control_bayes()` got a new argument `verbose_iter` that is used to control the verbosity of the Bayesian calculations. This change means that the `verbose` argument is being passed to `tune_grid()` to control its verbosity.
+
 
 * The `control_last_fit()` function gained an argument `allow_par` that defaults to `FALSE`. This change addresses failures after `last_fit()` using modeling engines that require native serialization, and we anticipate little to no increase in time-to-fit resulting from this change. (#539, tidymodels/bonsai#52)
 

R/checks.R

@@ -297,12 +297,36 @@ check_workflow <- function(x, pset = NULL, check_dials = FALSE) {
     }
   }
 
-  mod <- extract_spec_parsnip(x)
-  check_installs(mod)
+  check_extra_tune_parameters(x)
+
+  check_installs(hardhat::extract_spec_parsnip(x))
 
   invisible(NULL)
 }
 
+check_extra_tune_parameters <- function(x) {
+  mod <- hardhat::extract_spec_parsnip(x)
+
+  to_be_tuned <- hardhat::extract_parameter_set_dials(mod)
+  marked_for_tuning <- generics::tune_args(mod)
+
+  if (nrow(marked_for_tuning) > nrow(to_be_tuned)) {
+    not_tunable <- marked_for_tuning$name[!marked_for_tuning$name %in% to_be_tuned$name]
+    msg <-
+      c("!" = "{cli::qty(not_tunable)}The parameter{?s} {.var {not_tunable}}
+               {?was/were} marked with `tune()`, though will not be tuned.",
+        "i" = "This usually means that the current modeling engine
+               {.var {extract_spec_parsnip(x)$engine}}
+               does not support tuning {.var {not_tunable}}."
+      )
+
+
+    cli::cli_abort(msg, call = rlang::caller_env(3), class = "not_tunable_error")
+  }
+  invisible(NULL)
+}
+
+
 #' @export
 #' @keywords internal
 #' @rdname empty_ellipses

tests/testthat/_snaps/checks.md

@@ -84,6 +84,24 @@
       Error in `tune:::check_workflow()`:
       ! A parsnip model is required.
 
+# workflow objects (will not tune, tidymodels/tune#548)
+
+    Code
+      tune_grid(lr_lm_1, rec_bare, rsample::bootstraps(Chicago, 2))
+    Condition
+      Error in `tune_grid()`:
+      ! The parameter `penalty` was marked with `tune()`, though will not be tuned.
+      i This usually means that the current modeling engine `lm` does not support tuning `penalty`.
+
+---
+
+    Code
+      tune_bayes(lr_lm_2, rec_tune, rsample::bootstraps(Chicago, 2))
+    Condition
+      Error in `tune_bayes()`:
+      ! The parameters `penalty` and `mixture` were marked with `tune()`, though will not be tuned.
+      i This usually means that the current modeling engine `lm` does not support tuning `penalty` and `mixture`.
+
 # yardstick objects
 
     Code

tests/testthat/test-checks.R

@@ -179,6 +179,66 @@ test_that("workflow objects", {
   })
 })
 
+test_that("workflow objects (will not tune, tidymodels/tune#548)", {
+  skip_if_not_installed("glmnet")
+
+  # one recipe without tuning, one with:
+  rec_bare <- recipes::recipe(ridership ~ ., data = head(Chicago, 30))
+  rec_tune <- rec_bare %>% recipes::step_ns(temp_max, deg_free = tune())
+
+  # well-defined:
+  lr_lm_0 <- parsnip::linear_reg()
+
+  # not well-defined:
+  lr_lm_1 <- parsnip::linear_reg(penalty = tune())
+  lr_lm_2 <- parsnip::linear_reg(penalty = tune(), mixture = tune())
+
+  # well-defined:
+  lr_glmnet_0 <- lr_lm_0 %>% set_engine("glmnet")
+  lr_glmnet_1 <- lr_lm_1 %>% set_engine("glmnet")
+  lr_glmnet_2 <- lr_lm_2 %>% set_engine("glmnet")
+
+  # don't error when supplied tune args make sense given engine / steps
+  expect_error_na <- function(x) {testthat::expect_error(x, regexp = NA)}
+
+  expect_error_na(check_workflow(workflow(rec_bare, lr_lm_0)))
+  expect_error_na(check_workflow(workflow(rec_bare, lr_glmnet_0)))
+  expect_error_na(check_workflow(workflow(rec_bare, lr_glmnet_1)))
+  expect_error_na(check_workflow(workflow(rec_bare, lr_glmnet_2)))
+
+  expect_error_na(check_workflow(workflow(rec_tune, lr_lm_0)))
+  expect_error_na(check_workflow(workflow(rec_tune, lr_glmnet_0)))
+  expect_error_na(check_workflow(workflow(rec_tune, lr_glmnet_1)))
+  expect_error_na(check_workflow(workflow(rec_tune, lr_glmnet_2)))
+
+  # error when supplied tune args don't make sense given engine / steps
+  expect_error_nt <- function(x) {testthat::expect_error(x, class = "not_tunable_error")}
+
+  expect_error_nt(check_workflow(workflow(rec_bare, lr_lm_1)))
+  expect_error_nt(check_workflow(workflow(rec_bare, lr_lm_2)))
+
+  expect_error_nt(check_workflow(workflow(rec_tune, lr_lm_1)))
+  expect_error_nt(check_workflow(workflow(rec_tune, lr_lm_2)))
+
+  # ensure that error (incl `call`) is displayed as it ought to be:
+  expect_snapshot(
+    error = TRUE,
+    tune_grid(
+      lr_lm_1,
+      rec_bare,
+      rsample::bootstraps(Chicago, 2)
+    )
+  )
+  expect_snapshot(
+    error = TRUE,
+    tune_bayes(
+      lr_lm_2,
+      rec_tune,
+      rsample::bootstraps(Chicago, 2)
+    )
+  )
+})
+
 # ------------------------------------------------------------------------------
 
 test_that("yardstick objects", {