tidymodels / dials

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

respect inclusive fields in real-valued parameters #347

Open topepo opened 1 month ago

topepo commented 1 month ago

Per @simonpcouch's report:

library(dials)
#> Loading required package: scales
library(dplyr)
#> 
#> Attaching package: 'dplyr'
#> The following objects are masked from 'package:stats':
#> 
#>     filter, lag
#> The following objects are masked from 'package:base':
#> 
#>     intersect, setdiff, setequal, union

foo <- function(range = c(0, 1), trans = NULL) {
  dials::new_quant_param(
    type = "double",
    range = range,
    inclusive = c(FALSE, FALSE),
    trans = trans,
    label = c(foo = "bar"),
    finalize = NULL
  )
}

foo() %>% 
  value_seq(10) %>% 
  range()
#> [1] 0 1

# Same is true here but it is difficult to observe due to sampling: 
foo() %>% 
  value_sample(100) %>% 
  range()
#> [1] 0.01505507 0.99933483

Created on 2024-08-05 with reprex v2.1.0

The issue appears to be the use of

seq(from = min(unlist(object$range)), to = max(unlist(object$range)), length.out = n)

We should check the inclusive field and add/subtract .Machine$double.eps (about 2.2e-16) when inclusive = FALSE.

This would affect value_seq_dbl() and value_sample_dbl() and maybe their integer counterparts (with +/- 1L).