FIX nested backend setting n_jobs=-1#785
Conversation
Codecov Report
@@ Coverage Diff @@
## master #785 +/- ##
==========================================
+ Coverage 94.69% 95.27% +0.58%
==========================================
Files 44 44
Lines 6274 6307 +33
==========================================
+ Hits 5941 6009 +68
+ Misses 333 298 -35
Continue to review full report at Codecov.
|
joblib/test/test_parallel.py
Outdated
|
|
||
| with Parallel() as p: | ||
| with Parallel(n_jobs=2) as p: | ||
| print(p._backend) |
joblib/parallel.py
Outdated
| with parallel_backend(self._backend): | ||
| # Set the default nested backend to self._backend but do not set the | ||
| # change the default number of processes to -1 | ||
| with parallel_backend(self._backend, n_jobs=None): |
There was a problem hiding this comment.
I am worried this might disable nested parallelism for backends that expect
with parallel_backend('dask', n_jobs=-1):
do_stuff()Ideally, all the Parallel calls in the do_stuff function and nested calls should inherit the n_jobs=-1.
I am not sure if we can handle this properly.
There was a problem hiding this comment.
dask is the only backend where this would make sense. Typically, you don't want the nested backend to use n_jobs=-1 as it leads to massive over-subscription.
A way to deal with this is to have get_nested_backend return both a backend and n_jobs. That way, only dask will return n_jobs=-1 and others can have n_jobs=None.
|
@tomMoral I just pushed a new test to show what is the expected behavior with the dask backend. |
ogrisel
left a comment
There was a problem hiding this comment.
LGTM but we need an entry to the change log.
* tag '0.13.0': Release 0.13.0 Use https (joblib#805) MTN bump loky to 2.4.2 (joblib#804) DOC: provide some useful variables for Makefile (joblib#794) DOC serialization and processes (joblib#803) ENH update loky 2.4.0 (joblib#802) FIX backward compatibility for nested backend (joblib#789) enable python 3.7 (joblib#795) memory: add test for call_and_shelve performance (joblib#791) FIX nested backend not changed by SequentialBackend (joblib#792) cloudpickle 0.6.0 (joblib#788) FIX nested backend setting n_jobs=-1 (joblib#785) Raises a more explicit exception when a corrupted MemorizedResult is loaded (joblib#768) Back to dev mode
* releases: Release 0.13.0 Use https (joblib#805) MTN bump loky to 2.4.2 (joblib#804) DOC: provide some useful variables for Makefile (joblib#794) DOC serialization and processes (joblib#803) ENH update loky 2.4.0 (joblib#802) FIX backward compatibility for nested backend (joblib#789) enable python 3.7 (joblib#795) memory: add test for call_and_shelve performance (joblib#791) FIX nested backend not changed by SequentialBackend (joblib#792) cloudpickle 0.6.0 (joblib#788) FIX nested backend setting n_jobs=-1 (joblib#785) Raises a more explicit exception when a corrupted MemorizedResult is loaded (joblib#768) Back to dev mode
Fix for #784