Skip to content

Commit

Permalink
More debugging and tweaking for futures
Browse files Browse the repository at this point in the history
- Make sure the "future"/globalenv() testing scenario actually uses the global environment.
- Repair a testing scenario: "future" backend with globalenv(). Need to pass globals explicitly in a named list rather than let `future` just guess at the values.
- Use a clumsy workaround to make sure globals passed to the "future" backend do not conflict with anything `drake` needs. When #296 is solved, the point will be moot.
- Set future.globals = FALSE in `future_lapply()` to reduce overhead. This is possible since the "future_lapply" backend assumes file system access and relies completely on the cache.
  • Loading branch information
wlandau-lilly committed Mar 18, 2018
1 parent 7b5cd70 commit 1d4c825
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 9 deletions.
33 changes: 27 additions & 6 deletions R/future.R
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,28 @@ new_worker <- function(id, target, config, protect){
)){
return(empty_worker(target = target))
}
config$cache$flush_cache() # Less data to pass this way.
# Avoid potential name conflicts with other globals.
# When we solve #296, the need for such a clumsy workaround
# should go away.
globals <- list(
DRAKE_GLOBALS__ = list(
target = target,
meta = meta,
config = config,
protect = protect
)
)
if (identical(config$envir, globalenv())){
globals <- ls(config$envir) # Unit tests should modify global env # nocov
} else {
globals <- NULL
# Unit tests should not modify global env # nocov
if (exists("DRAKE_GLOBALS__", config$envir)){ # nocov # nolint
warning( # nocov
"Do not define an object named `DRAKE_GLOBALS__` ", # nocov
"in the global environment", # nocov
call. = FALSE # nocov
) # nocov
} # nocov
globals <- c(globals, as.list(config$envir, all.names = TRUE)) # nocov
}
evaluator <- drake_plan_override(
target = target,
Expand All @@ -81,13 +99,16 @@ new_worker <- function(id, target, config, protect){
) %||%
future::plan("next")
announce_build(target = target, meta = meta, config = config)
config$cache$flush_cache() # Less data to pass this way.
structure(
future::future(
expr = drake_future_task(
target = target, meta = meta, config = config, protect = protect),
target = DRAKE_GLOBALS__$target,
meta = DRAKE_GLOBALS__$meta,
config = DRAKE_GLOBALS__$config,
protect = DRAKE_GLOBALS__$protect
),
packages = "drake",
globals = c(globals, "target", "meta", "config", "protect"),
globals = globals,
evaluator = evaluator
),
target = target
Expand Down
3 changes: 2 additions & 1 deletion R/future_lapply.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ worker_future_lapply <- function(targets, meta_list, config){
x = targets,
FUN = build_distributed,
cache_path = config$cache$driver$path,
meta_list = meta_list
meta_list = meta_list,
future.globals = FALSE
)
}
4 changes: 2 additions & 2 deletions inst/testing/scenarios.csv
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ envir,parallelism,jobs,backend,caching
"global","Makefile",2,,
"global","future_lapply",1,"future::multicore",
"global","future_lapply",1,"future::multisession",
"local","future",2,"future::multicore","worker"
"local","future",9,"future::multisession","master"
"global","future",2,"future::multicore","worker"
"global","future",9,"future::multisession","master"

0 comments on commit 1d4c825

Please sign in to comment.