Change from Pool to ProcessPoolExecutor #1664
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1664 +/- ##
===========================================
- Coverage 84.37% 84.28% -0.09%
===========================================
Files 164 164
Lines 14320 14324 +4
===========================================
- Hits 12082 12073 -9
- Misses 2238 2251 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dweindl
left a comment
dweindl
left a comment
There was a problem hiding this comment.
I have the indexing in since we are not using imap anymore. Do we need a tests that sees we are returning them in the right oder? Probably not necessary to me, as we are anyways sorting them shortly after.
The MultiProcessEngine is not restricted to multi-start optimization, so it shouldn't rely on that and we should ensure that result ordering corresponds to task ordering. So yes, having a test would be good. Doesn't have to be any kind of optimization, can be some trivial task.
| ) -> list[Any]: | ||
| """Pickle tasks and distribute work over parallel processes. | ||
| Tasks are pickled on-demand as workers become available. |
There was a problem hiding this comment.
I don't think that's true. ProcessPoolExecutor.submit is non-blocking.
If it were true, the progress bar handling wouldn't work, because it would only start after the tasks have been submitted.
Closes #1499?
No. For closing that, after submitting the initial n_procs tasks, a new tasks should only be submitted after a previous one finished.
Nothing against merging this nonetheless, but please keep the other issue open.
Changed to ProcessPoolExecutor according to "concurrent.futures" documentation.
I have the indexing in since we are not using imap anymore. Do we need a tests that sees we are returning them in the right oder? Probably not necessary to me, as we are anyways sorting them shortly after.
Closes #1499?