Conversation
Codecov Report
@@ Coverage Diff @@
## master #1256 +/- ##
==========================================
+ Coverage 88.38% 88.49% +0.10%
==========================================
Files 47 50 +3
Lines 7052 7118 +66
==========================================
+ Hits 6233 6299 +66
Misses 819 819
Continue to review full report at Codecov.
|
tomMoral
left a comment
tomMoral
left a comment
There was a problem hiding this comment.
Looks nice with the test! thx @GaelVaroquaux
|
Thanks! It's indeed better to address this on the joblib side rather than in loky. |
ogrisel
left a comment
ogrisel
left a comment
There was a problem hiding this comment.
LGTM as well. Thanks for the fix.
I pushed a merge of master to run the tests with the updated CI config. There are still some failures on master but that should run the tests on more plaforms.
|
Please document the change in the changelog before merging. |
GaelVaroquaux
force-pushed
the
no_multiprocessing
branch
from
d6e6c1f to
d87e620
Compare
|
I rebased on master. Will document the changes |
|
OK, I have added the functionality to fall back to threading when multiprocessing is not available. There are still some failing tests, but I suspect that they are failing on master, so I will rebase on master when they are fixed. |
GaelVaroquaux
force-pushed
the
no_multiprocessing
branch
from
486bed5 to
fdf5ef4
Compare
GaelVaroquaux
force-pushed
the
no_multiprocessing
branch
from
fdf5ef4 to
3cbf561
Compare
|
I sync'ed master to check the CI. |
ogrisel
left a comment
ogrisel
left a comment
There was a problem hiding this comment.
There is an actual problem in the warning: see the failed CI.
Also, the coverage report indicates that test_backend_no_multiprocessing is never executed on our CI. Indeed I am not sure when this test is supposed to be executed. Have you run it locally with pyiodine?
If so do you think we should have a new CI config for this? Maybe it's not worth the extra maintenance effort and test_missing_multiprocessing is enough?
Co-authored-by: Olivier Grisel <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]>
|
The |
It works as it should, and actually captured legit errors which I fixed. Let's see what this gives. |
|
All green! (took me a while). This should be ready to merge. |
|
Merged. Thanks very much @hoodmane and @GaelVaroquaux! |
|
Thanks @GaelVaroquaux for completing this! Much appreciated. |
Release 1.2.0 Fix a security issue where eval(pre_dispatch) could potentially run arbitrary code. Now only basic numerics are supported. joblib/joblib#1327 Make sure that joblib works even when multiprocessing is not available, for instance with Pyodide joblib/joblib#1256 Avoid unnecessary warnings when workers and main process delete the temporary memmap folder contents concurrently. joblib/joblib#1263 Fix memory alignment bug for pickles containing numpy arrays. This is especially important when loading the pickle with mmap_mode != None as the resulting numpy.memmap object would not be able to correct the misalignment without performing a memory copy. This bug would cause invalid computation and segmentation faults with native code that would directly access the underlying data buffer of a numpy array, for instance C/C++/Cython code compiled with older GCC versions or some old OpenBLAS written in platform specific assembly. joblib/joblib#1254 Vendor cloudpickle 2.2.0 which adds support for PyPy 3.8+. Vendor loky 3.3.0 which fixes several bugs including: robustly forcibly terminating worker processes in case of a crash (joblib/joblib#1269); avoiding leaking worker processes in case of nested loky parallel calls; reliability spawn the correct number of reusable workers.
5 participants
A continuation of #1246 to make joblib robust to non working multiprocessing.