`check_future_plan()` throws an error because the `workers` name/slot is no longer provided #447

njtierney · 2021-11-02T09:44:32Z

https://github.com/greta-dev/greta/blob/master/R/utils.R#L899-L904

This code expects there to be a workers name/slot in the future object, but there is not.

This then breaks the tests at https://github.com/greta-dev/greta/blob/master/tests/testthat/test_inference.R#L477-L551

f_thing <- future::future(NULL, lazy = TRUE)

names(f_thing)
#>  [1] "label"         "local"         "owner"         "envir"        
#>  [5] "packages"      "gc"            "conditions"    "expr"         
#>  [9] "uuid"          "seed"          "version"       "result"       
#> [13] "asynchronous"  "calls"         "globals"       "stdout"       
#> [17] "earlySignal"   "lazy"          "state"         ".defaultLocal"

f_thing$workers
#> NULL

^{Created on 2021-11-02 by the reprex package (v2.0.1)}

Session info

sessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#>  setting  value                       
#>  version  R version 4.1.1 (2021-08-10)
#>  os       macOS Big Sur 10.16         
#>  system   x86_64, darwin17.0          
#>  ui       X11                         
#>  language (EN)                        
#>  collate  en_AU.UTF-8                 
#>  ctype    en_AU.UTF-8                 
#>  tz       Australia/Perth             
#>  date     2021-11-02                  
#> 
#> ─ Packages ───────────────────────────────────────────────────────────────────
#>  package     * version date       lib source        
#>  backports     1.2.1   2020-12-09 [1] CRAN (R 4.1.0)
#>  cli           3.1.0   2021-10-27 [1] CRAN (R 4.1.1)
#>  codetools     0.2-18  2020-11-04 [1] CRAN (R 4.1.1)
#>  crayon        1.4.1   2021-02-08 [1] CRAN (R 4.1.0)
#>  digest        0.6.28  2021-09-23 [1] CRAN (R 4.1.0)
#>  ellipsis      0.3.2   2021-04-29 [1] CRAN (R 4.1.0)
#>  evaluate      0.14    2019-05-28 [1] CRAN (R 4.1.0)
#>  fansi         0.5.0   2021-05-25 [1] CRAN (R 4.1.0)
#>  fastmap       1.1.0   2021-01-25 [1] CRAN (R 4.1.0)
#>  fs            1.5.0   2020-07-31 [1] CRAN (R 4.1.0)
#>  future        1.23.0  2021-10-31 [1] CRAN (R 4.1.1)
#>  globals       0.14.0  2020-11-22 [1] CRAN (R 4.1.0)
#>  glue          1.4.2   2020-08-27 [1] CRAN (R 4.1.0)
#>  highr         0.9     2021-04-16 [1] CRAN (R 4.1.0)
#>  htmltools     0.5.2   2021-08-25 [1] CRAN (R 4.1.0)
#>  knitr         1.36    2021-09-29 [1] CRAN (R 4.1.0)
#>  lifecycle     1.0.1   2021-09-24 [1] CRAN (R 4.1.0)
#>  listenv       0.8.0   2019-12-05 [1] CRAN (R 4.1.0)
#>  magrittr      2.0.1   2020-11-17 [1] CRAN (R 4.1.0)
#>  parallelly    1.28.1  2021-09-09 [1] CRAN (R 4.1.0)
#>  pillar        1.6.4   2021-10-18 [1] CRAN (R 4.1.0)
#>  pkgconfig     2.0.3   2019-09-22 [1] CRAN (R 4.1.0)
#>  purrr         0.3.4   2020-04-17 [1] CRAN (R 4.1.0)
#>  R.cache       0.15.0  2021-04-30 [1] CRAN (R 4.1.0)
#>  R.methodsS3   1.8.1   2020-08-26 [1] CRAN (R 4.1.0)
#>  R.oo          1.24.0  2020-08-26 [1] CRAN (R 4.1.0)
#>  R.utils       2.11.0  2021-09-26 [1] CRAN (R 4.1.0)
#>  reprex        2.0.1   2021-08-05 [1] CRAN (R 4.1.0)
#>  rlang         0.4.12  2021-10-18 [1] CRAN (R 4.1.0)
#>  rmarkdown     2.11    2021-09-14 [1] CRAN (R 4.1.0)
#>  rstudioapi    0.13    2020-11-12 [1] CRAN (R 4.1.0)
#>  sessioninfo   1.1.1   2018-11-05 [1] CRAN (R 4.1.0)
#>  stringi       1.7.5   2021-10-04 [1] CRAN (R 4.1.0)
#>  stringr       1.4.0   2019-02-10 [1] CRAN (R 4.1.0)
#>  styler        1.6.2   2021-09-23 [1] CRAN (R 4.1.0)
#>  tibble        3.1.5   2021-09-30 [1] CRAN (R 4.1.0)
#>  utf8          1.2.2   2021-07-24 [1] CRAN (R 4.1.0)
#>  vctrs         0.3.8   2021-04-29 [1] CRAN (R 4.1.0)
#>  withr         2.4.2   2021-04-18 [1] CRAN (R 4.1.0)
#>  xfun          0.26    2021-09-14 [1] CRAN (R 4.1.0)
#>  yaml          2.2.1   2020-02-01 [1] CRAN (R 4.1.0)
#> 
#> [1] /Library/Frameworks/R.framework/Versions/4.1/Resources/library

The text was updated successfully, but these errors were encountered:

HenrikBengtsson · 2021-11-02T14:23:51Z

The reason why this "hack" no longer works is that in future (>= 1.22.0), a lazy future has not yet been "metamorphosed" into its final form (according to the current plan(). The quick fix, still a hack though, is to use lazy = FALSE.

HenrikBengtsson · 2021-11-02T15:47:23Z

This then breaks the tests at https://github.com/greta-dev/greta/blob/master/tests/testthat/test_inference.R#L477-L551

Is that a new test? I ran reverse package dependency checks with future 1.23.0 on 2021-10-27 and, except from a teeny NOTE, nothing came up, cf. https://github.com/HenrikBengtsson/future/blob/develop/revdep/problems.md#greta.

njtierney · 2021-11-03T01:17:14Z

Thanks, @HenrikBengtsson !

I'll need to check in with @goldingn on this part of the future check code, but so far that fix has resolved that test file, thank you!

Yes, that wouldn't have been picked up on reverse pkg dependency checks with future as we end up needing to skip a lot of tests as they either take too long to run on CRAN, or we cannot get CRAN to have a tensorflow setup.

We had the tests running successfully on GH actions but they are a bit fragile. I will try and rig up a cron job for this so we can stay on top of things!

Note to self: make greta import version 1.22.0 of future.

…lisation of chains. (changing to `future::future(NULL, lazy = FALSE)` instead of `lazy = TRUE`. See greta-dev#447 for more details. Also alphabetised the Imports and Suggests packages

HenrikBengtsson · 2021-11-03T01:46:25Z

Yes, that wouldn't have been picked up on reverse pkg dependency checks with future as we end up needing to skip a lot of tests as they either take too long to run on CRAN, or we cannot get CRAN to have a tensorflow setup.

Would it be possible to pull that piece of code into a help function that you can run tests on?

BTW, I you still want to condition the code on a workers element actually existing, e.g. if (!is.null(workers)) .... As I mentioned in HenrikBengtsson/future#556, not all types of futures have an internal workers element (regardless of lazy = TRUE/FALSE).

The actual objective, of being able to fall back to sequential process when workers are forked processes, will be address in future via a new "resource" specification framework, e.g.

f <- future({ ... }, resources = !fork)

It's on the near roadmap but it'll probably be another 6-12 months before there is a working prototype.

goldingn · 2021-11-03T06:25:23Z

Thanks @HenrikBengtsson!
Yes, makes sense. That 'workers' code was fragile and only meant as a temporary fix.
Testing the functions that interface with future separately is a good idea.

HenrikBengtsson · 2021-11-03T14:37:19Z

As an alternative to assert that against forked processing upfront, could you just assert this at the first step of your future expression? If so, you could use parallelly::isForkedChild(). Proof of concept:

my_fcn <- function() {
  ## WORKAROUND: https://github.com/HenrikBengtsson/parallelly/issues/71
  dummy <- parallelly::isForkedChild()

  f <- future({ 
    if (parallelly::isForkedChild()) {
      stop("Does not support forked parallelization")
    }
  
    42 
  })
  value(f)
}

For example,

library(future)

plan(sequential)
my_fcn()
#> [1] 42

plan(multisession)
my_fcn()
#> [1] 42

cl <- parallel::makeCluster(2)
plan(cluster, workers = cl)
my_fcn()
#> [1] 42

cl <- parallel::makeCluster(2, type = "FORK")
plan(cluster, workers = cl)
my_fcn()
#> Error in withCallingHandlers({ : Does not support forked parallelization

plan(multicore)
my_fcn()
#> Error in withCallingHandlers({ : Does not support forked parallelization

HenrikBengtsson · 2021-11-03T14:44:25Z

I discovered a bug in parallelly::isForkedChild() requiring a small workaround until resolved (code edited in above comment).

njtierney · 2021-11-04T03:39:46Z

Thanks again for this, @HenrikBengtsson !

I don't get the same error at plan(multicore) as you have, not sure if I'm missing something:

library(future)

my_fcn <- function() {
  ## WORKAROUND: https://github.com/HenrikBengtsson/parallelly/issues/71
  dummy <- parallelly::isForkedChild()
  
  f <- future({ 
    if (parallelly::isForkedChild()) {
      stop("Does not support forked parallelization")
    }
    
    42 
  })
  value(f)
}

plan(sequential)
my_fcn()
#> [1] 42

plan(multisession)
my_fcn()
#> [1] 42

cl <- parallel::makeCluster(2)
plan(cluster, workers = cl)
my_fcn()
#> [1] 42

cl <- parallel::makeCluster(2, type = "FORK")
plan(cluster, workers = cl)
my_fcn()
#> Error in withCallingHandlers({: Does not support forked parallelization

plan(multicore)
my_fcn()
#> [1] 42


cl <- parallel::makeForkCluster(2L)
plan(cluster, workers = cl)
my_fcn()
#> Error in withCallingHandlers({: Does not support forked parallelization

^{Created on 2021-11-04 by the reprex package (v2.0.1)}

Session info

sessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#>  setting  value                       
#>  version  R version 4.1.1 (2021-08-10)
#>  os       macOS Big Sur 10.16         
#>  system   x86_64, darwin17.0          
#>  ui       X11                         
#>  language (EN)                        
#>  collate  en_AU.UTF-8                 
#>  ctype    en_AU.UTF-8                 
#>  tz       Australia/Perth             
#>  date     2021-11-04                  
#> 
#> ─ Packages ───────────────────────────────────────────────────────────────────
#>  package     * version date       lib source        
#>  backports     1.2.1   2020-12-09 [1] CRAN (R 4.1.0)
#>  cli           3.1.0   2021-10-27 [1] CRAN (R 4.1.1)
#>  codetools     0.2-18  2020-11-04 [1] CRAN (R 4.1.1)
#>  crayon        1.4.1   2021-02-08 [1] CRAN (R 4.1.0)
#>  digest        0.6.28  2021-09-23 [1] CRAN (R 4.1.0)
#>  ellipsis      0.3.2   2021-04-29 [1] CRAN (R 4.1.0)
#>  evaluate      0.14    2019-05-28 [1] CRAN (R 4.1.0)
#>  fansi         0.5.0   2021-05-25 [1] CRAN (R 4.1.0)
#>  fastmap       1.1.0   2021-01-25 [1] CRAN (R 4.1.0)
#>  fs            1.5.0   2020-07-31 [1] CRAN (R 4.1.0)
#>  future      * 1.23.0  2021-10-31 [1] CRAN (R 4.1.1)
#>  globals       0.14.0  2020-11-22 [1] CRAN (R 4.1.0)
#>  glue          1.4.2   2020-08-27 [1] CRAN (R 4.1.0)
#>  highr         0.9     2021-04-16 [1] CRAN (R 4.1.0)
#>  htmltools     0.5.2   2021-08-25 [1] CRAN (R 4.1.0)
#>  knitr         1.36    2021-09-29 [1] CRAN (R 4.1.0)
#>  lifecycle     1.0.1   2021-09-24 [1] CRAN (R 4.1.0)
#>  listenv       0.8.0   2019-12-05 [1] CRAN (R 4.1.0)
#>  magrittr      2.0.1   2020-11-17 [1] CRAN (R 4.1.0)
#>  parallelly    1.28.1  2021-09-09 [1] CRAN (R 4.1.0)
#>  pillar        1.6.4   2021-10-18 [1] CRAN (R 4.1.0)
#>  pkgconfig     2.0.3   2019-09-22 [1] CRAN (R 4.1.0)
#>  purrr         0.3.4   2020-04-17 [1] CRAN (R 4.1.0)
#>  R.cache       0.15.0  2021-04-30 [1] CRAN (R 4.1.0)
#>  R.methodsS3   1.8.1   2020-08-26 [1] CRAN (R 4.1.0)
#>  R.oo          1.24.0  2020-08-26 [1] CRAN (R 4.1.0)
#>  R.utils       2.11.0  2021-09-26 [1] CRAN (R 4.1.0)
#>  reprex        2.0.1   2021-08-05 [1] CRAN (R 4.1.0)
#>  rlang         0.4.12  2021-10-18 [1] CRAN (R 4.1.0)
#>  rmarkdown     2.11    2021-09-14 [1] CRAN (R 4.1.0)
#>  rstudioapi    0.13    2020-11-12 [1] CRAN (R 4.1.0)
#>  sessioninfo   1.1.1   2018-11-05 [1] CRAN (R 4.1.0)
#>  stringi       1.7.5   2021-10-04 [1] CRAN (R 4.1.0)
#>  stringr       1.4.0   2019-02-10 [1] CRAN (R 4.1.0)
#>  styler        1.6.2   2021-09-23 [1] CRAN (R 4.1.0)
#>  tibble        3.1.5   2021-09-30 [1] CRAN (R 4.1.0)
#>  utf8          1.2.2   2021-07-24 [1] CRAN (R 4.1.0)
#>  vctrs         0.3.8   2021-04-29 [1] CRAN (R 4.1.0)
#>  withr         2.4.2   2021-04-18 [1] CRAN (R 4.1.0)
#>  xfun          0.26    2021-09-14 [1] CRAN (R 4.1.0)
#>  yaml          2.2.1   2020-02-01 [1] CRAN (R 4.1.0)
#> 
#> [1] /Library/Frameworks/R.framework/Versions/4.1/Resources/library

…greta-dev#447

njtierney · 2021-11-04T10:08:28Z

I think I worked this out!

HenrikBengtsson · 2021-11-04T17:02:58Z

I think I worked this out!

Good. The thing is that, for the parallelly::isForkedChild() workaround to work, it has to be done before the user calls plan(multicore), not after. So, it's not 100% right now. You could call parallelly::isForkedChild() in .onLoad() to lower the risk for not working.

This has been fixed in the next release of parallelly (HenrikBengtsson/parallelly#71), so you don't have to do any workarounds later on. Alternatively, to get it right already now, you'd need to use internal parallel:::isChild().

HenrikBengtsson · 2021-11-21T06:33:59Z

FYI, parallelly 1.29.0 is now on CRAN. With that, you can use parallelly::isForkedChild() without any workarounds.

njtierney · 2021-11-23T02:37:14Z

Thanks so much, @HenrikBengtsson :)

Just to check in on this, do you think I need to put parallelly::isForkedChild() into .onLoad() now?

Or should the following work?

test_if_forked_cluster <- function(){

  f <- future({
    if (parallelly::isForkedChild()) {
      msg <- cli::format_error(
        "parallel mcmc samplers cannot be run with a fork cluster"
      )
      stop(
        msg,
        call. = FALSE
      )
    }

    42
  })

  value(f)
}

Which is then called in:

  # if running in parallel
  if (plan_is$parallel) {

    # if it's a cluster, check there's no forking
    if (plan_is$cluster) {

      test_if_forked_cluster()

      f <- future::future(NULL, laze = FALSE)
# more code from here:...

HenrikBengtsson · 2021-11-23T03:53:30Z

Just to check in on this, do you think I need to put parallelly::isForkedChild() into .onLoad() now?

There's no longer a need for that.

Or should the following work?

Yes, like that. Or, if you want to produce the error in the main R process;

test_if_forked_cluster <- function(){
   is_forked <- value(future(parallelly::isForkedChild()))
    if (is_forked) {
      msg <- cli::format_error(
        "parallel mcmc samplers cannot be run with a fork cluster"
      )
      stop(
        msg,
        call. = FALSE
      )
    }
}

njtierney mentioned this issue Nov 3, 2021

setup GH actions to run on a cron job #448

Closed

njtierney mentioned this issue Nov 3, 2021

Test functions that interface with future separately #449

Closed

njtierney added this to the 0.3.2 milestone Nov 3, 2021

njtierney added the Up Next label Nov 3, 2021

njtierney mentioned this issue Nov 3, 2021

Fix future check plan 447 #451

Merged

njtierney closed this as completed in #451 Nov 4, 2021

njtierney added a commit to njtierney/greta that referenced this issue Nov 4, 2021

trying out improved checking of forks using Henrik's suggested code in …

043e38d

…greta-dev#447

njtierney reopened this Nov 4, 2021

njtierney mentioned this issue Nov 4, 2021

trying out improved checking of forks using Henrik's suggested code i… #455

Merged

njtierney closed this as completed Nov 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`check_future_plan()` throws an error because the `workers` name/slot is no longer provided #447

`check_future_plan()` throws an error because the `workers` name/slot is no longer provided #447

njtierney commented Nov 2, 2021

HenrikBengtsson commented Nov 2, 2021 •

edited

Loading

HenrikBengtsson commented Nov 2, 2021

njtierney commented Nov 3, 2021

HenrikBengtsson commented Nov 3, 2021

goldingn commented Nov 3, 2021

HenrikBengtsson commented Nov 3, 2021 •

edited

Loading

HenrikBengtsson commented Nov 3, 2021

njtierney commented Nov 4, 2021

njtierney commented Nov 4, 2021

HenrikBengtsson commented Nov 4, 2021 •

edited

Loading

HenrikBengtsson commented Nov 21, 2021

njtierney commented Nov 23, 2021

HenrikBengtsson commented Nov 23, 2021

check_future_plan() throws an error because the workers name/slot is no longer provided #447

check_future_plan() throws an error because the workers name/slot is no longer provided #447

Comments

njtierney commented Nov 2, 2021

HenrikBengtsson commented Nov 2, 2021 • edited Loading

HenrikBengtsson commented Nov 2, 2021

njtierney commented Nov 3, 2021

HenrikBengtsson commented Nov 3, 2021

goldingn commented Nov 3, 2021

HenrikBengtsson commented Nov 3, 2021 • edited Loading

HenrikBengtsson commented Nov 3, 2021

njtierney commented Nov 4, 2021

njtierney commented Nov 4, 2021

HenrikBengtsson commented Nov 4, 2021 • edited Loading

HenrikBengtsson commented Nov 21, 2021

njtierney commented Nov 23, 2021

HenrikBengtsson commented Nov 23, 2021

`check_future_plan()` throws an error because the `workers` name/slot is no longer provided #447

`check_future_plan()` throws an error because the `workers` name/slot is no longer provided #447

HenrikBengtsson commented Nov 2, 2021 •

edited

Loading

HenrikBengtsson commented Nov 3, 2021 •

edited

Loading

HenrikBengtsson commented Nov 4, 2021 •

edited

Loading