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

map and pmap to fix #32 #108

Merged
merged 5 commits into from
Sep 29, 2018
Merged

map and pmap to fix #32 #108

merged 5 commits into from
Sep 29, 2018

Conversation

zsunberg
Copy link
Collaborator

This PR will add support for both map and pmap

Primary usage will be through @showprogress, e.g.

@showprogress pmap(x->x^2, 1:10)

I have implemented the progress_map function, which uses channels to pass progress information back to the main process that prints the progress meter. It works for both map and pmap. Now I just need to modify showprogress to convert map and pmap calls into progress_map calls.

Comments welcome.

@zsunberg
Copy link
Collaborator Author

Ok, I think this is nearly ready to merge.

There is one problem - currently ProgressMeter has to be loaded on all processes for this to work. I believe this is because the anonymous function defined on line 575 apparently is defined only in the ProgressMeter module. I am not sure how to fix it.

I have put a fair amount of effort into creating concise code that works in all the cases I could think of, so I hope it can get merged soon :)

@zsunberg zsunberg changed the title [WIP] map and pmap to fix #32 map and pmap to fix #32 Aug 16, 2018
@djsegal
Copy link

djsegal commented Sep 25, 2018

Is there an end in sight for this? Would be really useful to have it working.

It doesn't have to be perfect, for example

has worked in every place I've ever used it?

@zsunberg
Copy link
Collaborator Author

Not sure what the cruel joke is - this PR is v0.7 compatible. Just need a maintainer to merge it @timholy . I think there might be another package that uses the builtin julia logging that people are switching to.

@zsunberg
Copy link
Collaborator Author

@garrison are you the right one to ask to merge this?

@timholy timholy merged commit b493d55 into timholy:master Sep 29, 2018
@timholy
Copy link
Owner

timholy commented Sep 29, 2018

Sorry for the unconscionable delay, I have too many packages that I maintain and need to ditch responsibility for some. I'll give more people rights to this one.

@timholy
Copy link
Owner

timholy commented Sep 29, 2018

Turns out there were 5 people already authorized to merge PRs here, but I added you too @zsunberg.

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

Successfully merging this pull request may close these issues.

3 participants