[WIP] Disables the loky backend for environments where sem_open implementation is not available#999
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #999 +/- ##
==========================================
+ Coverage 92.26% 95.32% +3.05%
==========================================
Files 45 45
Lines 6645 6648 +3
==========================================
+ Hits 6131 6337 +206
+ Misses 514 311 -203 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| from multiprocessing import synchronize | ||
| synchronize.SemLock._make_name = staticmethod(_make_name) | ||
| except ImportError as import_error: | ||
| # sys.stderr.write(f"Disabling loky support: {import_error}") |
There was a problem hiding this comment.
I'm not certain how this should be handled. Where is it best to log this error?
There was a problem hiding this comment.
Why is this needed if joblib/parallel.py already uses a try / except ImportError on from .externals import loky?
|
I'm waiting for the tests to pass before I rebase this. |
|
https://travis-ci.org/joblib/joblib/jobs/642093480?utm_medium=notification&utm_source=github_status is failing with the following error: I am not certain as to what is triggering this, but the only other test which was failing was a flake8 issue I resolved. The other tests in the build pass: |
have a functioning sem_open implementation; Moving cloudpickle_wrapper (this is not dependent upon loky)
6a49f2b to
8aa38c5
Compare
loky backend for environments where sem_open implementation is not availableloky backend for environments where sem_open implementation is not available
|
Do you mind adding a test future releases would not break this? |
loky backend for environments where sem_open implementation is not availableloky backend for environments where sem_open implementation is not available
ogrisel
left a comment
There was a problem hiding this comment.
I am fine defaulting to the threading backend on platforms that do not support sem_open but here are some comments:
|
|
||
| from .externals.loky import wrap_non_picklable_objects | ||
|
|
||
| from .externals.cloudpickle_wrapper import wrap_non_picklable_objects |
There was a problem hiding this comment.
This is problematic. The loky vendoring script will not work anymore and users would expect loky consistency will be lose.
I would rather wrap this with a try / except ImportError and not export the wrap_non_picklable_objects in __all__ when loky cannot be imported. Add a comment to explain that this can happen on systems that do not support sem_open (e.g. Android).
| from multiprocessing import synchronize | ||
| synchronize.SemLock._make_name = staticmethod(_make_name) | ||
| except ImportError as import_error: | ||
| # sys.stderr.write(f"Disabling loky support: {import_error}") |
There was a problem hiding this comment.
Why is this needed if joblib/parallel.py already uses a try / except ImportError on from .externals import loky?
|
Any news on this? |
Resolves #825, but I need to add an alternative
wrap_non_picklable_objectsimplementation and change the error logging a bit.