-
Notifications
You must be signed in to change notification settings - Fork 38
POC: Signal progress via progressr #305
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
base: main
Are you sure you want to change the base?
Conversation
|
Yes, this all makes sense to me. Providing a Also, analogously to the new ys <- furrr::future_map(xs, fcn) |> progressify()
ys <- purrr::map(xs, fcn) |> progressify() |> futurize()can simply transpile info: ys <- furrr::future_map(xs, fcn, .progress = TRUE)
Agree - this is tricky. I've been hoping something would emerge over time. If a |
|
@HenrikBengtsson I'm a bit worried about merging this without some kind of proactive advise to users about the change. I'd love any thoughts on what you think a nice upgrade path would be. Here's my main worry. This used to emit a progress bar without any additional code: library(furrr)
plan(multisession, workers = 2)
xs <- seq_len(100)
model_fn <- function(x) {
Sys.sleep(.1)
x
}
out <- future_map(xs, model_fn, .progress = TRUE)That will no longer emit a progress bar, and this change will be completely silent from a user's perspective. Users will have to either:
i.e. they need to do this now library(furrr)
# THIS IS NEW. It must be in your script somewhere or in an `.Rprofile` if you always want progressr updates
progressr::handlers(global = TRUE)
plan(multisession, workers = 2)
xs <- seq_len(100)
model_fn <- function(x) {
Sys.sleep(.1)
x
}
out <- future_map(xs, model_fn, .progress = TRUE)
But how do we communicate this change to them? Here's a few ideas!
|
|
Oooh here's another option that I do actually like quite a bit! First let's note that furrr (and future.apply) live in an interesting space:
So my proposal is that furrr functions have two progress modes:
This seems like it would work quite nicely! For end users:
For developers:
|
|
I agree the path forward with this change in behavior is not obvious. My suggestion would be:
I've got a working A parallel task would be to work with R Core (Luke) to allow for registering global condition handlers on package load (futureverse/progressr#78). Somewhere, maybe in person, I think Luke mentioned that he might be open to it. A conservative approach could be to have namespace-specific global calling handlers so that packages couldn't step on each other and overwrite each others handlers. That would make it possible for progressr to make |
I'm a bit surprised to hear that you'd be open to that. I thought you wanted users to have to explicitly opt in to visual progress notifications. But this suggests that you recognize that If this was the default then |
Yes, I'm thinking opt-out instead of opt-in. It's also important that only progressr would be allow to register this (i.e. no other packages) to avoid competing interests - I guess that goes for all global calling handlers. We lack standards for this in R, so it's a bit "exploratory". We already have R option I might disagree with myself on all this in the future ¯_(ツ)_/¯ |
Closes #138
Closes #263
This implements the ideas in futureverse/progressr#85, all of the arguments there are still valid, and the global calling handler part of progressr makes this way easier to swallow as well.
TODO!in docs.progress = TRUE(for advanced cases only)handle_cli()slowness? It is very bad! Do notforcecli to refresh the display progressr#190@HenrikBengtsson this is an experimental ideal that emits
progressr::progressor()backed progress updates after each.finvocation when.progress = TRUE. We create aprogressr::progressor()in the main R process which has as many steps as there are elements to iterate over, and that is then exported to all of the workers. The workers just callp()after each.fcall.I know we've had various conversations about where the user vs developer split is on progress bars. I feel like this is actually a pretty reasonable solution, but I'm very interested to hear your thoughts!
with_progress()or (my preference)handlers(global = TRUE).handlers(handler_cli())and friends.progressr::progressor()2) call it at some interval.progress = FALSE/TRUEflag of their own, which their users should opt into for progression updated. I feel this is particularly important because progression signaling is definitely not "free" and should not really be used with short running tasks.I noticed that
handler_cli()is extremely slow. I see futureverse/progressr#167 and would love to see if we can get to the root of that. The progress bar in purrr doesn't seem to have this issue and is backed by cli. Something weird must be happening here.I imagine user scripts to look something like this