Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

develop: introduced bug #417

Closed
HenrikBengtsson opened this issue Sep 21, 2020 · 3 comments
Closed

develop: introduced bug #417

HenrikBengtsson opened this issue Sep 21, 2020 · 3 comments
Labels
Milestone

Comments

@HenrikBengtsson
Copy link
Owner

With future 1.18.0 the following works:

fcn <- function(...) future.apply::future_lapply(1:2, FUN = function(x) list(...))
> str(fcn(a=1, b=2))
List of 2
 $ :List of 2
  ..$ a: num 1
  ..$ b: num 2
 $ :List of 2
  ..$ a: num 1
  ..$ b: num 2

whereas with the develop branch, we get:

> fcn <- function(...) future.apply::future_lapply(1:2, FUN = function(x) list(...))
> str(fcn(a=1, b=2))
Error in ...future.FUN(...future.X_jj, ...) : 
  unused arguments (a = 1, b = 2)


Enter a frame number, or 0 to exit   

 1: str(fcn(a = 1, b = 2))
 2: fcn(a = 1, b = 2)
 3: #1: future.apply::future_lapply(1:2, FUN = function(x) list(...))
 4: future_xapply(FUN = FUN, nX = nX, chunk_args = X, args = list(...), get_chunk = `[`, expr = expr, envir = envir
 5: value(fs)
 6: value.list(fs)
 7: resolve(y, result = TRUE, stdout = stdout, signal = signal, force = TRUE)
 8: resolve.list(y, result = TRUE, stdout = stdout, signal = signal, force = TRUE)
 9: signalConditionsASAP(obj, resignal = FALSE, pos = ii)
10: signalConditions(obj, exclude = getOption("future.relay.immediate", "immediateCondition"), resignal = resignal,
11: stop(condition)

NOTE: This was spotted by CRAN incoming reverse-dependency checks. It was not spotted by revdepcheck (https://github.com/HenrikBengtsson/future/tree/develop/revdep) because this bug is only hit in the rangeMapper vignette, which revdepcheck skips by default. It's an unusual use of ... by rangeMapper but still valid. It would be better to pass ... down as an argument rather than relying on it being a global;

> fcn <- function(...) future.apply::future_lapply(1:2, FUN = function(x, ...) list(...), ...)
> str(fcn(a=1, b=2))
@HenrikBengtsson HenrikBengtsson added this to the 1.19.0 milestone Sep 21, 2020
@HenrikBengtsson
Copy link
Owner Author

Ah... this bug exists also in future 1.18.0, but only when using something else than sequential and multicore futures, e.g.

future::plan("cluster", workers = 1L)
fcn <- function(...) future.apply::future_lapply(1, FUN = function(x) list(...))
y <- fcn(a=1)
## Error in ...future.FUN(...future.X_jj, ...) : unused argument (a = 1)

The bug occurs also with sequential and multicore after 302b3f4 and 302b3f4, respectively.

@HenrikBengtsson
Copy link
Owner Author

HenrikBengtsson commented Sep 21, 2020

This boils down to:

future 1.18.0:

> expr
{
    ### irrelevant code            
    lapply(seq_along(...future.elements_ii), FUN = function(jj) {
        ...future.X_jj <- ...future.elements_ii[[jj]]
        NULL
        ...future.FUN(...future.X_jj, ...)
    })
}

future 1.19.0:

> gp$expr
{
    do.call(function(...) {
        ### irrelevant code            
        lapply(seq_along(...future.elements_ii), FUN = function(jj) {
            ...future.X_jj <- ...future.elements_ii[[jj]]
            NULL
            ...future.FUN(...future.X_jj, ...)
        })
    }, args = future.call.arguments)
}

str(gp$globals$future.call.arguments)
List of 1
 $ a: num 1
 - attr(*, "class")= chr [1:2] "DotDotDotList" "list"

(This seem awfully familiar to some old tricky ellipses case from 2016/2017; hopefully I'm wrong)


(Multitasking here, so just pasting in some quick notes in order not to forget): I think a simplified version of what's going on is:

> expr
lapply(1L, FUN = function(jj) ...future.FUN(jj, ...))
> gp$expr
do.call(function(...) {
  lapply(1L, FUN = function(jj) ...future.FUN(jj, ...))
}, args = future.call.arguments)

> gp$globals$...future.FUN
function(x) list(...)
<environment: 0x55efea66d678>

> names(environment(gp$globals$...future.FUN))
[1] "..."

BTW, there's also a duplicated future.call.arguments in there:

> names(gp$globals)
[1] "...future.FUN"             "future.call.arguments"     "future.call.arguments"     "...future.elements_ii"    
[5] "...future.seeds_ii"        "...future.globals.maxSize"

Browse[2]> str(gp$globals[[2]])
List of 1
 $ a: num 1
 - attr(*, "class")= chr [1:2] "DotDotDotList" "list"
NOTE: .Random.seed changed
Browse[2]> str(gp$globals[[3]])
 list()
 - attr(*, "class")= chr [1:2] "DotDotDotList" "list"

HenrikBengtsson added a commit to HenrikBengtsson/future.apply that referenced this issue Sep 21, 2020
@HenrikBengtsson
Copy link
Owner Author

HenrikBengtsson commented Sep 21, 2020

For the record, this bug would also happen with furrr, e.g.

fcn0 <- function(...) { purrr::map(1, function(x) list(...)) }
z0 <- fcn0(a = 1)
str(list(z0 = z0))

fcn <- function(...) { furrr::future_map(1, function(x) list(...)) }
z1 <- fcn(a = 1)
## Error in ...future.f(...future.x_jj, ...) : unused argument (a = 1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant