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

[RFC] add support for multiple simultaneous progressbars #157

Open
wants to merge 46 commits into
base: master
Choose a base branch
from

Conversation

MarcMush
Copy link
Collaborator

@MarcMush MarcMush commented May 25, 2020

following my answer in #156 , this adds two types of progressbars that mimic Progress:

ParallelProgress is pretty much the example in the readme and works like that :

using Distributed
addprocs()
@everywhere using ProgressMeter
prog = ParallelProgress(10; desc="test ")
pmap(1:10) do i
    sleep(rand())
    next!(prog)
    return myid()
end

MultipleProgress spawns multiple progress bars using offset

using Distributed
addprocs(2)
@everywhere using ProgressMeter
p = MultipleProgress([Progress(10; desc="task $i ") for i in 1:5], Progress(50; desc="global "))
pmap(1:5) do i
    for _ in 1:10
        sleep(rand())
        next!(p[i])
    end
    sleep(0.01)
    myid()
end
global 100%|██████████████████████████████████████████████████████████| Time: 0:00:24
task 4 100%|██████████████████████████████████████████████████████████| Time: 0:00:05
task 5 100%|██████████████████████████████████████████████████████████| Time: 0:00:05

It is also possible to add progressbars from another worker

p = MultipleProgress(Progress(10, "tasks done "); count_finishes=true)
sleep(0.1)
pmap(1:10) do i
    L = rand(20:50)
    p[i] = Progress(L, desc=" task $i ")
    for _ in 1:L
        sleep(0.05)
        next!(p[i])
    end
end

the main progress can be a Progress, either counting all individual steps, or counting finished tasks. It can also be a ProgressUnknown

the other progresses can be Progress, UnknownProgress or ProgressTresh

feel free to try this PR with ]add ProgressMeter#5c21f5d and give feedback and suggestions
Everything should be explained in ?MultipleProgress

closes #9, closes #96, closes #97, closes #125, closes #156, closes #253

@IanButterworth
Copy link
Collaborator

This is great! It's exactly what I was just looking for, and seems stable on a few trials across remote distributed workers.

One minor thought, I noticed that the color field of the added update!() method isn't used. It might be nice to be able to change that on the fly. I couldn't see an easy way to implement that with this approach.
I've also in the past updated the progress bar description during runs by changing prog.desc directly. It might be nice to add that functionality, but that could definitely wait for a future PR.

@IanButterworth
Copy link
Collaborator

Master now has the option to update the desc from update!() via #159. It would be good to think about whether it could be incorporated here. I did try a few attempts, but couldn't figure out a simple modification.

@MarcMush MarcMush changed the title [WIP] [RFC] add support for multiple simultaneous progressbars [RFC] add support for multiple simultaneous progressbars Jun 25, 2020
@MarcMush
Copy link
Collaborator Author

I added tests and fixed some bugs due to overshooting

@ianshmean it wouldn't be difficult to change it, by passing NamedTuples instead of Ints in the RemoteChannel

@MarcMush
Copy link
Collaborator Author

@jtrakk
Copy link

jtrakk commented Dec 5, 2020

This looks really nice. Does it work for tasks distributed over threads as well?

@MarcMush
Copy link
Collaborator Author

MarcMush commented Dec 7, 2020

yes, it should work, you can even use it on a single core
I didn't work on this much since I did this PR, but I can continue working on it if there is demand for it (or if someone wants to continue it, feel free to do it)

@jtrakk
Copy link

jtrakk commented Dec 22, 2020

I think it would be really useful for parallel long-running tasks. For example, it would be a nice improvement in the UX of parallel algorithms in Turing.jl.

@stevengj
Copy link
Contributor

stevengj commented Jun 1, 2021

It would be nicer if you could encapsulate any other progress bars, e.g. ProgressUnknown etcetera, in this way. That is, if you could just pass an array of progress meters:

p = MultipleProgress([Progress(...), ProgressUnknown(...), ProgressThresh(...)])

@bdeonovic
Copy link

Whats the status of this?

@MarcMush
Copy link
Collaborator Author

I did some work on this PR
I implemented @IanButterworth's suggestion #157 (comment)

@MarcMush
Copy link
Collaborator Author

It would be nicer if you could encapsulate any other progress bars, e.g. ProgressUnknown etcetera, in this way. That is, if you could just pass an array of progress meters:

p = MultipleProgress([Progress(...), ProgressUnknown(...), ProgressThresh(...)])

I really like the idea, and I enabled it for ParallelProgress, but for MultipleProgress, there will be a problem for tasks that are not started immediatly, since the start time will report when the whole object was created
There is several solutions to this, I don't know which one is better:

  • just don't care, the timings will be wrong in some cases
  • pass functions that will be executed at first call of the progressmeter eg
    p = MultipleProgress([()->Progress(...), ()->ProgressUnknown(...), ()->ProgressThresh(...)])
  • reset the p.tinit at first call

note that right now, and with one these solutions (except the first one) the first step won't be included in the timing except if initialized with update!(p,0) for example

@codecov
Copy link

codecov bot commented Aug 12, 2021

Codecov Report

Attention: Patch coverage is 98.19277% with 3 lines in your changes missing coverage. Please review.

Project coverage is 97.21%. Comparing base (807496a) to head (5c21f5d).
Report is 15 commits behind head on master.

Files Patch % Lines
src/parallel_progress.jl 98.19% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #157      +/-   ##
==========================================
+ Coverage   93.48%   97.21%   +3.72%     
==========================================
  Files           1        2       +1     
  Lines         399      717     +318     
==========================================
+ Hits          373      697     +324     
+ Misses         26       20       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MarcMush
Copy link
Collaborator Author

MarcMush commented Sep 3, 2023

I've changed to a Dict for storing the progressbars and throws an error when trying to re-add a progess with the same id. It is possible to use Ints or any other type for indexing. The main progress can be specified with the kwarg main and recalled with p.main

@Moelf
Copy link

Moelf commented Nov 7, 2023

this seems actually ready? gentle bump for a nod from main devs and we can do documentation etc. and ship it?

@MarcMush
Copy link
Collaborator Author

MarcMush commented Nov 7, 2023

have you tested it? I'd appreciate other people trying it and giving feedback, see if it is actually what is wanted
you can use it with ]add ProgressMeter#3156d04

@Moelf
Copy link

Moelf commented Nov 7, 2023

I read the examples here and I'm exactly using Distributed pmap so I think yes?

@MarcMush
Copy link
Collaborator Author

MarcMush commented Nov 7, 2023

@distributed pmap() was already implemented before

this PR adds next!() accross workers with ParallelProgress, as well as multiple progressbars at once with MultipleProgress

@Moelf
Copy link

Moelf commented Nov 7, 2023

@distributed pmap() was already implemented before

I have multiple sub tasks and also pmap() is not good enough because workers are unstable

@MarcMush
Copy link
Collaborator Author

MarcMush commented Nov 7, 2023

great, you can use this PR with ]add ProgressMeter#3156d04 and see if it works as expected for your usecase

 - `addprogress!` replaced with `setindex!`
 - add `ping(::ParallelProgress)` for testing
 - use `try_put!` for not erroring when over-shooting
 - remove `keys(::MultipleProgress)`
 - use of `waitfor` in tests for more intelligent waiting
 - use new keyword-only syntax
@MarcMush
Copy link
Collaborator Author

I changed the syntax for adding new progressbars, now it simply works with setindex!:

mp = MultipleProgress()
mp[1] = Progress(10)
next!(p[1])

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