BREAKING_CHANGE: predict timings are cumulative

NEWS.md

@@ -5,6 +5,8 @@
 * refactor: Optimize runtime of setting row roles.
 * refactor: Optimize runtime of marshalling.
 * refactor: Optimize runtime of `Task$col_info`
+* BREAKING CHANGE: the predict time of the learner now stores the cumulative duration for
+  all predict sets (#992).
 * feat: `$internal_valid_task` can now be set to an `integer` vector.
 * deprecated the `$divide()` method 
 * fix: `Task$cbind()` now works with non-standard primary keys

R/Learner.R

@@ -191,6 +191,9 @@ Learner = R6Class("Learner",
     #' This currently only works for methods `Learner$predict()` and `Learner$predict_newdata()`,
     #' and has no effect during [resample()] or [benchmark()] where you have other means
     #' to parallelize.
+    #'
+    #' Note that the recorded time required for prediction reports the time required to predict
+    #' is not properly defined and depends on the parallelization backend.
     parallel_predict = FALSE,
 
     #' @field timeout (named `numeric(2)`)\cr
@@ -346,6 +349,10 @@ Learner = R6Class("Learner",
         }
       }, add = TRUE)
 
+      # reset learner predict time; this is only cumulative for multiple predict sets,
+      # not for multiple calls to predict / predict_newdata
+      self$state$predict_time = 0
+
       # we only have to marshal here for the parallel prediction case, because learner_predict() handles the
       # marshaling for call-r encapsulation itself
       if (isTRUE(self$parallel_predict) && nbrOfWorkers() > 1L) {
@@ -360,7 +367,6 @@ Learner = R6Class("Learner",
         pdata = learner_predict(self, task, row_ids)
       }
 
-
       if (is.null(pdata)) {
         return(NULL)
       } else {
@@ -461,6 +467,11 @@ Learner = R6Class("Learner",
 
     #' @field timings (named `numeric(2)`)\cr
     #' Elapsed time in seconds for the steps `"train"` and `"predict"`.
+    #'
+    #' When predictions for multiple predict sets were made during [resample()] or [benchmark()],
+    #' the predict time shows the cumulative duration of all predictions.
+    #' If `learner$predict()` is called manually, the last predict time gets overwritten.
+    #'
     #' Measured via [mlr3misc::encapsulate()].
     timings = function(rhs) {
       assert_ro_binding(rhs)

R/MeasureElapsedTime.R

@@ -12,6 +12,10 @@
 #' Aggregation of elapsed time defaults to mean but can be configured via the field `aggregator` of the
 #' [Measure].
 #'
+#' When predictions for multiple predict sets were made during [resample()] or [benchmark()],
+#' the predict time shows the cumulative duration of all predictions.
+#' If `learner$predict()` is called manually, the last predict time gets overwritten.
+#'
 #' @template param_id
 #' @templateVar id time_train
 #' @template measure

R/worker.R

@@ -210,7 +210,7 @@ learner_predict = function(learner, task, row_ids = NULL) {
 
     pdata = result$result
     learner$state$log = append_log(learner$state$log, "predict", result$log$class, result$log$msg)
-    learner$state$predict_time = result$elapsed
+    learner$state$predict_time = sum(learner$state$predict_time, result$elapsed)
 
     lg$debug("Learner '%s' returned an object of class '%s'",
       learner$id, class(pdata)[1L], learner = learner$clone(), prediction_data = pdata, messages = result$log$msg)

tests/testthat/test_Learner.R

@@ -490,7 +490,7 @@ test_that("compatability check on validation task", {
   task$internal_valid_task$col_roles$target = "credit_history"
   expect_error(learner$train(task), "has different target")
 })
-  
+
 test_that("model is marshaled during parallel predict", {
   # by setting check_pid = TRUE, we ensure that unmarshal_model() sets the process id to the current
   # id. LearnerClassifDebug then checks during `.predict()`, whether the marshal_id of the model is equal to the current process id and errs if this is not the case.
@@ -540,7 +540,7 @@ test_that("learner state contains internal valid task information", {
   expect_string(rr$learners[[1L]]$state$internal_valid_task_hash)
 
   # 1. manual
-  learner$train(task) 
+  learner$train(task)
   expect_string(learner$state$internal_valid_task_hash)
 })
 
@@ -550,3 +550,14 @@ test_that("validation task with 0 observations", {
   task$internal_valid_task = integer(0)
   expect_error({learner$train(task)}, "has 0 observations")
 })
+
+test_that("predict time is cumulative", {
+  learner = lrn("classif.debug", sleep_predict = function() 0.05)
+  task = tsk("iris")
+  learner$train(task)$predict(task)
+  t1 = learner$timings["predict"]
+  learner$param_set$values$sleep_predict = function() 0.01
+  learner$predict(task)
+  t2 = learner$timings["predict"]
+  expect_true(t1 > t2)
+})