-
Notifications
You must be signed in to change notification settings - Fork 424
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.
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
No multiprocessing #1256
No multiprocessing #1256
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.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks nice with the test! thx @GaelVaroquaux
Thanks! It's indeed better to address this on the joblib side rather than in loky. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
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. |
486bed5
to
fdf5ef4
Compare
fdf5ef4
to
3cbf561
Compare
I sync'ed master to check the CI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
A continuation of #1246 to make joblib robust to non working multiprocessing.