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

ENH: Annotate mris_expand with thread usage #386

Merged
merged 2 commits into from
Nov 18, 2023

Conversation

effigies
Copy link
Member

This one's a little weird, due to the multiple inheritance and also the inefficiency of multithreading in mris_expand.

I noticed midthickness taking a long time while running tests, and I was pretty sure I'd seen that it could be parallelized.

From watching htop and running mris_expand with a set of OMP_NUM_THREADS values, I see that generally you get about 65-75% core utilization, generally a bit under. We therefore overallocate threads by about 50% to make the core usage match the desired number of threads, occasionally peaking over but mostly sitting a bit under. This works pretty well for 2-12 desired threads (3-18 OMP_NUM_THREADS), but seems almost completely limited after that, with 24 threads utilizing maybe 13 cores at best. Therefore, I set this to max out at 12 for scheduling purposes.

The memory usage of this tool is low and does not seem to increase noticeably with number of threads.

If n_procs == 1, str(1 * 3 // 2) == '1', so this does not interfere with people attempting to run things single-threaded.

Copy link

codecov bot commented Nov 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b5e8663) 53.47% compared to head (43eed0e) 58.48%.

❗ Current head 43eed0e differs from pull request most recent head 899f2f4. Consider uploading reports for the commit 899f2f4 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             next     #386      +/-   ##
==========================================
+ Coverage   53.47%   58.48%   +5.00%     
==========================================
  Files          24       27       +3     
  Lines        1840     1910      +70     
  Branches      239      240       +1     
==========================================
+ Hits          984     1117     +133     
+ Misses        787      736      -51     
+ Partials       69       57      -12     
Flag Coverage Δ
ds005 ∅ <ø> (?)
ds054 45.99% <71.42%> (+0.06%) ⬆️
pytest 37.12% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@effigies
Copy link
Member Author

If there are no objections, I'll merge this before a release. It should reduce a 20min job to 2.5-4 minutes.

@effigies effigies added the next label Nov 17, 2023
@effigies effigies merged commit 6ce2121 into nipreps:next Nov 18, 2023
13 of 14 checks passed
@effigies effigies deleted the enh/multithread-mrisexpand branch November 18, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant