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

add support for using @showprogress on Threads.@threads for loops #284

Merged
merged 5 commits into from
Feb 8, 2024

Conversation

Fro116
Copy link
Contributor

@Fro116 Fro116 commented Nov 14, 2023

This lets you use the @showprogress macro on parallel for loops. For example,

@showprogress Threads.@threads for i in 1:100
    sleep(0.1)
end

Copy link

codecov bot commented Nov 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5b3bd1d) 96.81% compared to head (0ca12d5) 97.03%.
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
+ Coverage   96.81%   97.03%   +0.22%     
==========================================
  Files           1        1              
  Lines         533      539       +6     
==========================================
+ Hits          516      523       +7     
+ Misses         17       16       -1     

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

@Fro116
Copy link
Contributor Author

Fro116 commented Nov 14, 2023

My bad, I never tested on older versions of Julia. The Threads.@threads had a different expression tree before v1.3.0-rc1 and so the new test I added was failing on older builds.

I'll gate the Threads.@threads test behind a @static if VERSION >= v"1.3.0-rc1" statement, the same way we do for some Threads.@Spawn tests in test_threads.jl.

@Fro116
Copy link
Contributor Author

Fro116 commented Nov 14, 2023

Take three. I updated to fix the code coverage errors.

The last run failed on the nightly ubuntu-latest build because the test case "test_threads.jl ProgressUnknown" errored out. I'm not sure why. That test should not touch any of the code in this pr. I can't repro it, and it didn't fail in the first run (https://github.com/timholy/ProgressMeter.jl/actions/runs/6859117251/job/18660114093?pr=284). Is the test flaky on master?

@MarcMush
Copy link
Collaborator

Recently yes
Ref #281

@Fro116
Copy link
Contributor Author

Fro116 commented Nov 16, 2023

Okay, afaict this passes everything except for the flaky test. Should be good to go, but let me know if there's anything else I should look into.

Copy link
Collaborator

@MarcMush MarcMush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

support for @Threads.threads is very welcome!

src/ProgressMeter.jl Outdated Show resolved Hide resolved
src/ProgressMeter.jl Outdated Show resolved Hide resolved
test/test.jl Outdated Show resolved Hide resolved
@Fro116 Fro116 requested a review from MarcMush February 2, 2024 04:52
@MarcMush
Copy link
Collaborator

MarcMush commented Feb 2, 2024

LGTM

@MarcMush MarcMush merged commit 6575743 into timholy:master Feb 8, 2024
14 checks passed
@MarcMush MarcMush mentioned this pull request Mar 4, 2024
@MarcMush MarcMush mentioned this pull request Jul 13, 2024
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.

2 participants