Increase convergence tolerance for root finding

[jdblischak]

I implemented the idea of increasing the convergence tolerance. This PR is a draft because currently I don't think this is worth pursuing. The median speed improvement on my machine is consistently about 20 milliseconds compared to the current code. And unfortunately it breaks many of the unit tests, even after I also adjusted the tolerance in the internal functions used in the test comparisons. Thus this idea would require more effort for at best a minimal improvement.

library("microbenchmark")

remotes::install_github("Merck/gsDesign2@6d4371a", INSTALL_opts = "--with-keep.source")
## Skipping install of 'gsDesign2' from a github remote, the SHA1 (6d4371ae) has not changed since last install.
##   Use `force = TRUE` to force installation
library("gsDesign2")
packageVersion("gsDesign2")
## [1] ‘1.1.0.1’

microbenchmark(
  gs_power_ahr = gs_power_ahr(
    analysis_time = c(12, 24, 36),
    event = c(30, 40, 50),
    binding = TRUE,
    upper = gs_spending_bound,
    upar = list(sf = gsDesign::sfLDOF, total_spend = 0.025, param = NULL, timing = NULL),
    lower = gs_spending_bound,
    lpar = list(sf = gsDesign::sfLDOF, total_spend = 0.025, param = NULL, timing = NULL)
  ),
  times = 100
)
## Unit: milliseconds
##          expr      min       lq     mean   median       uq      max neval
##  gs_power_ahr 376.1468 403.4486 450.8562 434.6752 489.9425 688.7457   100

detach("package:gsDesign2")
devtools::load_all()
## ℹ Loading gsDesign2
packageVersion("gsDesign2")
## [1] ‘1.1.0.2’

microbenchmark(
  gs_power_ahr = gs_power_ahr(
    analysis_time = c(12, 24, 36),
    event = c(30, 40, 50),
    binding = TRUE,
    upper = gs_spending_bound,
    upar = list(sf = gsDesign::sfLDOF, total_spend = 0.025, param = NULL, timing = NULL),
    lower = gs_spending_bound,
    lpar = list(sf = gsDesign::sfLDOF, total_spend = 0.025, param = NULL, timing = NULL)
  ),
  times = 100
)
## Unit: milliseconds
##          expr      min       lq     mean   median       uq      max neval
##  gs_power_ahr 354.4254 383.3973 430.2495 422.6296 459.8087 647.3404   100

[nanxstats]

tol = 1e-6 was previously used in gsDesign::gsDesign() and the subsequent gsbound C routines... so maybe better to keep it consistent? 🤔

Tolerance for error (default is 0.000001). Normally this will not be changed by the user. This does not translate directly to number of digits of accuracy, so use extra decimal places.

[jdblischak]

Closing this now. This PR has served as documentation of the approach. Given the minimal speed improvement, I don't think it is worth the required effort of updating the tests and documentation

[jdblischak]

tol = 1e-6 was previously used in gsDesign::gsDesign() and the subsequent gsbound C routines... so maybe better to keep it consistent? 🤔

I think this is a separate tolerance. Both gs_power_ahr() (and the old function gs_power_ahr_()) have the argument tol = 1e-6

gsDesign2/R/gs_power_ahr.R

Line 173 in 6d4371a

tol = 1e-6,

This then gets passed to gs_power_npe()

gsDesign2/R/gs_power_ahr.R

Lines 201 to 207 in 6d4371a

	y_h1 <- gs_power_npe(
	theta = x$theta,
	info = x$info, info0 = x$info0, info1 = x$info, info_scale = info_scale,
	upper = upper, upar = upar, test_upper = test_upper,
	lower = lower, lpar = lpar, test_lower = test_lower,
	binding = binding, r = r, tol = tol
	)

But the tol argument is not passed to gs_info_ahr() (which doesn't have a tol argument), which is what then subsequently calls expected_time() (which also does not have a tol argument)

gsDesign2/R/gs_power_ahr.R

Line 194 in 6d4371a

x <- gs_info_ahr(

[jdblischak]

xref: #219