8000 More elegant multithreading handling · Issue #401 · ME-ICA/tedana · GitHub
[go: up one dir, main page]
Skip to content

More elegant multithreading handling #401 8000

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.

Sign up for GitHub

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

Jump to bottom
Closed
1 task
jbteves opened this issue Sep 24, 2019 · 3 comments · Fixed by #537
Closed
1 task

More elegant multithreading handling #401

jbteves opened this issue Sep 24, 2019 · 3 comments · Fixed by #537
Labels
priority: high issues that would be really helpful if they were fixed already refactoring issues proposing/requesting changes to the code which do not impact behavior

Comments

@jbteves
Copy link
Collaborator
jbteves commented Sep 24, 2019 •
edited
Loading

Summary

Since we last managed environment variables, numpy has begun recommending that people use the threadpoolctl package. We should add this package as a dependency and use it to manage the multithreading environment.

Next Steps

  • add threadpoolctl to this project
@jbteves jbteves mentioned this issue Sep 24, 2019
Closed
@tsalo tsalo added the refactoring issues proposing/requesting changes to the code which do not impact behavior label Oct 4, 2019
@tsalo tsalo added the priority: high issues that would be really helpful if they were fixed already label Nov 20, 2019
@tsalo
Copy link
Member
tsalo commented Nov 20, 2019

Bumping this up in priority per user concerns in #473.

@tsalo tsalo mentioned this issue Nov 23, 2019
Closed
@jbteves
Copy link
Collaborator Author
jbteves commented Dec 23, 2019

So I forgot on the conference call that indeed, the only way to set environment variables for multithreading is with os.environ or threadpoolctl. Therefore, there are actually two solutions:

  1. Use threadpoolctl to control thread count in the tedana workflows call
  2. Retrieve the os environment values and replace them at the conclusion of the workflow

The second is less safe because a failed run will result in the user environment variables being changed, so I'm going to work with the first. However, there is a point of failure in that you can only choose one user API to set the thread count for, so we have to pick one. We could later add an additional argument to pick the API from BLAS, MKL, etc. See threadpoolctl code on the matter here. For now, I'll open a PR to handle the most common case of BLAS, unless anyone objects.

@handwerkerd handwerkerd mentioned this issue Jan 10, 2020
Closed
@tsalo tsalo mentioned this issue Feb 21, 2020
Merged
@tsalo
Copy link
Member
tsalo commented Feb 21, 2020

I just want to say that this issue is related to #188 and #215 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
priority: high issues that would be really helpful if they were fixed already refactoring issues proposing/requesting changes to the code which do not impact behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[ENH] Control number of threads with threadpoolctl tsalo/tedana
2 participants
@tsalo @jbteves
0