RevolutionAnalytics / foreach

R package to provide foreach looping construct
Apache License 2.0
53 stars 4 forks source link

.noexport not working as expected #34

Open mitokic opened 2 years ago

mitokic commented 2 years ago

Hi there,

The .noexport value within the foreach function doesn't seem to work correctly when both the function to loop over and the call to foreach itself live within various functions. Please see the example below.

library(dplyr)
library(foreach)
library(timetk)

# function to call foreach and run in parallel
submit_par <- function(iterator, fn) {

  cl <- parallel::makeCluster(3)
  doParallel::registerDoParallel(cl)

  temp <- foreach::foreach(i = iterator, 
                           .combine = 'rbind',
                           .packages = c("dplyr"),
                           .export = NULL, 
                           .errorhandling = "stop", 
                           .verbose = FALSE, 
                           .inorder = FALSE, 
                           .multicombine = TRUE, 
                           .noexport = c("combos")
  ) %dopar% {fn(i)}

  parallel::stopCluster(cl)

  return(temp)
}

# main function that takes some data and calls foreach
outer_fn <- function(input_tbl) {

  data <- input_tbl %>%
    dplyr::filter(date > "2010-01-01")

  combos <- unique(data$id)

  par_fn <- function(i) {

    df <- data %>%
      dplyr::filter(id == i)

    return(exists("combos"))
  }

  output <- submit_par(combos, par_fn)

  return(output)
}

# call to function with some example data
outer_fn(timetk::m4_monthly)

Here is what is returned from the outer_fn call

         [,1]
result.1 TRUE
result.2 TRUE
result.3 TRUE
result.4 TRUE

I'm expecting to see all FALSE values, since the object "combos" was specified in the ".noexport" argument so it shouldn't be exported to the cluster. But it turns out it is being exported. Is there anything I need to change with the above functions to ensure certain objects are not exported? I've played around with removing objects from specific environments within the "outer_fn" environment before calling foreach but that turns into a slippery slope if those objects need to be used in any way after the foreach call.

Thanks for your help! I'm trying to migrate my CRAN package (finnts) to run on spark using sparklyr. And there is a limit to the amount of data that can be serialized when running foreach in spark. So it's necessary I can easily remove specific objects from getting exported to compute clusters (either through doparallel or sparklyr).

bwlewis commented 2 years ago

'combos' in this example is captured in the environment of the function 'fn', which needs to be exported. Thus it comes along for the ride with fn. Here is a somewhat more minimal example without the package dependencies that shows this behavior:

library(foreach)

# function to call foreach and run in parallel
submit_par <- function(iterator, fn) {
  cl <- parallel::makeCluster(1)

print(ls(environment(fn)))

  doParallel::registerDoParallel(cl)
  temp <- foreach::foreach(i = iterator,
                           .combine = "rbind",
                           .export = NULL,
                           .errorhandling = "stop",
                           .verbose = TRUE,
                           .inorder = FALSE,
                           .multicombine = TRUE,
                           .noexport = "combos"
  ) %dopar% {fn(i)}
  parallel::stopCluster(cl)
  return(temp)
}

# main function that calls foreach
outer_fn <- function() {
  combos <- pi
  par_fn <- function(i) {
    return(exists("combos"))
  }
  output <- submit_par(pi, par_fn)
  return(output)
}

print(outer_fn())

I guess you can in this case manually remove combos from that function environment here.

More broadly, it is true that the .noexport option does not recursively interrogate all environments to ban exporting names; it only works at the top level. Perhaps the doc should be changed to make that more explicit?

EDIT:

Also, FYI, bascially same behavior in future, for instance:

library(future)
plan("future::multisession")
submit_par <- function(fn) {
  f <- future({fn()}, globals = list(fn = fn))
  value(f)
}
outer_fun <- function() {
  combos <- pi
  par_fn <- function() {
    return(combos)
  }
  submit_par(par_fn)
}
outer_fun()

Exporting the function fn in submit_par also exports the environment tied to fn. You'll have to manually prune that to prevent large values from going over the wire.

bwlewis commented 2 years ago

Also, FYI, bascially same behavior in future, for instance:

library(future)
plan("future::multisession")
submit_par <- function(fn) {
  f <- future({fn()}, globals = list(fn = fn))
  value(f)
}
outer_fun <- function() {
  combos <- pi
  par_fn <- function() {
    return(combos)
  }
  submit_par(par_fn)
}
outer_fun()

Exporting the function fn in submit_par also exports the environment tied to fn. You'll have to manually prune that to prevent large values from going over the wire.

On 5/7/22, Bryan Lewis @.***> wrote:

'combos' in this example is captured in the environment of the function 'fn', which needs to be exported. Thus it comes along for the ride with fn. Here is a somewhat more minimal example without the package dependencies that shows this behavior:

library(foreach)

# function to call foreach and run in parallel
submit_par <- function(iterator, fn) {
  cl <- parallel::makeCluster(1)

print(ls(environment(fn)))

  doParallel::registerDoParallel(cl)
  temp <- foreach::foreach(i = iterator,
                           .combine = "rbind",
                           .export = NULL,
                           .errorhandling = "stop",
                           .verbose = TRUE,
                           .inorder = FALSE,
                           .multicombine = TRUE,
                           .noexport = "combos"
  ) %dopar% {fn(i)}
  parallel::stopCluster(cl)
  return(temp)
}

# main function that calls foreach
outer_fn <- function() {
  combos <- pi
  par_fn <- function(i) {
    return(exists("combos"))
  }
  output <- submit_par(pi, par_fn)
  return(output)
}

print(outer_fn())

I guess you can in this case manually remove combos from that function environment here.

More broadly, it is true that the .noexport option does not recursively interrogate all environments to ban exporting names; it only works at the top level. Perhaps the doc should be changed to make that more explicit?

On 5/5/22, Mike Tokic @.***> wrote:

Hi there,

The .noexport value within the foreach function doesn't seem to work correctly when both the function to loop over and the call to foreach itself live within various functions. Please see the example below.

library(dplyr)
library(foreach)
library(timetk)

# function to call foreach and run in parallel
submit_par <- function(iterator, fn) {

  cl <- parallel::makeCluster(3)
  doParallel::registerDoParallel(cl)

  temp <- foreach::foreach(i = iterator,
                           .combine = 'rbind',
                           .packages = c("dplyr"),
                           .export = NULL,
                           .errorhandling = "stop",
                           .verbose = FALSE,
                           .inorder = FALSE,
                           .multicombine = TRUE,
                           .noexport = c("combos")
  ) %dopar% {fn(i)}

  parallel::stopCluster(cl)

  return(temp)
}

# main function that takes some data and calls foreach
outer_fn <- function(input_tbl) {

  data <- input_tbl %>%
    dplyr::filter(date > "2010-01-01")

  combos <- unique(data$id)

  par_fn <- function(i) {

    df <- data %>%
      dplyr::filter(id == i)

    return(exists("combos"))
  }

  output <- submit_par(combos, par_fn)

  return(output)
}

# call to function with some example data
outer_fn(timetk::m4_monthly)

Here is what is returned from the outer_fn call

         [,1]
result.1 TRUE
result.2 TRUE
result.3 TRUE
result.4 TRUE

I'm expecting to see all FALSE values, since the object "combos" was specified in the ".noexport" argument so it shouldn't be exported to the cluster. But it turns out it is being exported. Is there anything I need to change with the above functions to ensure certain objects are not exported? I've played around with removing objects from specific environments within the "outer_fn" environment before calling foreach but that turns into a slippery slope if those objects need to be used in any way after the foreach call.

Thanks for your help! I'm trying to migrate my CRAN package (finnts) to run on spark using sparklyr. And there is a limit to the amount of data that can be serialized when running foreach in spark. So it's necessary I can easily remove specific objects from getting exported to compute clusters (either through doparallel or sparklyr).

-- Reply to this email directly or view it on GitHub: https://github.com/RevolutionAnalytics/foreach/issues/34 You are receiving this because you are subscribed to this thread.

Message ID: @.***>