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.

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

Fix the calculation for the number of threads to be used #39

Merged
merged 4 commits into from
Sep 26, 2024
Merged

Fix the calculation for the number of threads to be used #39

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
13e59ef
[impl]: Fix calculation for the number of threads to be used
haoyu-zc Sep 24, 2024
9dfea76
[impl]: Factor out logic to compute the number of threads/processes t…
haoyu-zc Sep 26, 2024
fd1cb1c
[impl]: Replace assertions with exceptions
haoyu-zc Sep 26, 2024
65cd7bf
[doc]: Add missing docstring
haoyu-zc Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libs/ccc/coef/impl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ def ccc(

# get number of cores to use
n_jobs = os.cpu_count() if n_jobs is None else n_jobs
default_n_threads = (os.cpu_count() - n_jobs) if n_jobs < 0 else n_jobs
default_n_threads = (os.cpu_count() + n_jobs) if n_jobs < 0 else n_jobs
Copy link
Member

Choose a reason for hiding this comment

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

This fixes the problem described in the PR. However, if n_jobs has an absolute value equal or larger than os.cpu_count() it will be zero or negative, right? We should check if the final value for default_n_threads is within a valid range.

Copy link
Member

Choose a reason for hiding this comment

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

And I think the docstring of this function needs to be updated, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes that's what I missed! I'll update the code and add test cases. Thanks!


if internal_n_clusters is not None:
_tmp_list = List()
Expand Down