Fix the calculation for the number of threads to be used

[haoyu-zc]

There's a bug when n_jobs is provided as a negative number. Here's the original contract and code:

Docstring:
n_jobs: number of CPU cores to use for parallelization. The value
None will use all available cores (os.cpu_count()), and negative
values will use os.cpu_count() - n_jobs. Default is 1.

Code:

# get the 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

It's clear that we should fix the subtraction to an addition:

default_n_threads = (os.cpu_count() + n_jobs) if n_jobs < 0 else n_jobs

[miltondp]

Thank you, @haoyu-zc. I left some comments requesting a few more changes. See if it makes sense for you.

[miltondp]

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.

[miltondp]

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

[haoyu-zc]

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

[haoyu-zc]

Changes made:

• Renamed default_n_threads to n_workers since now we can use both processes and threads
• Wrote a function to compute the number of workers, covered by unit tests
• Replaced two assertions with exceptions

[miltondp]

Looks great. Should we also update the version in setup.py for a new release, or you plan to do that in another PR? (whatever you prefer is fine for me).

Also, after installing this version (pip install -e .), I tested again the last issue #37 (with Python 3.12, not Python 3.9) and now everything works as expected.

Another issue that seems to be fixed now is #38 :) I wonder what happened.

[miltondp]

Looks good. If a user specifies more threads than actual CPU cores, that's a problem for the user (using more resources than actual ones) and we should not check for that.

[haoyu-zc]

Yep! As ThreadPoolExecutor(max_workers=n_workers) also only complains when n_workers < 0. We don't need to check for that case

[haoyu-zc]

Looks great. Should we also update the version in setup.py for a new release, or you plan to do that in another PR? (whatever you prefer is fine for me).

Also, after installing this version (pip install -e .), I tested again the last issue #37 (with Python 3.12, not Python 3.9) and now everything works as expected.

Another issue that seems to be fixed now is #38 :) I wonder what happened.

I think it might be some changes with Python itself. I don't have a clue now either...

I'll do a new PR for the new release to clarify things.