Skip to content
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

Joblib.Parallel hangs when passed an empty iterable (python 2.7) #292

Closed
joshloyal opened this issue Jan 4, 2016 · 10 comments
Closed

Joblib.Parallel hangs when passed an empty iterable (python 2.7) #292

joshloyal opened this issue Jan 4, 2016 · 10 comments
Labels
bug

Comments

@joshloyal
Copy link

@joshloyal joshloyal commented Jan 4, 2016

Currently the parallel loop will hang for the following code snippets

from math import sqrt
from joblib import Parallel, delayed

Parallel(n_jobs=2)(delayed(sqrt)(i) for i in range(0))

or even just

Parallel(n_jobs=2)([])

I'm not too familiar with the internals of the parallel loop, but it seems to be because we are requesting a parallel loop with an empty iterable (it works fine for n_jobs=1, i.e. a flat loop). Up until joblib==0.9.2 this would return an empty list.

@joshloyal joshloyal changed the title Joblib.Parallel hangs when passed an empty iterable Joblib.Parallel hangs when passed an empty iterable (python 2.7) Jan 4, 2016
@lesteve

This comment has been minimized.

Copy link
Member

@lesteve lesteve commented Jan 5, 2016

Can reproduce on both python 2 and python 3.

@lesteve lesteve added the bug label Jan 5, 2016
@aabadie

This comment has been minimized.

Copy link
Contributor

@aabadie aabadie commented Jan 5, 2016

I could make this work by simply adding self._iterating = False here: https://github.com/joblib/joblib/blob/master/joblib/parallel.py#L720

Otherwise the caller is stuck in an infinite loop.

@lesteve

This comment has been minimized.

Copy link
Member

@lesteve lesteve commented Jan 5, 2016

I could make this work by simply adding self._iterating = False here: https://github.com/joblib/joblib/blob/master/joblib/parallel.py#L720

Otherwise the caller is stuck in an infinite loop.

This doesn't feel like the right place to do it but I may be wrong. I double-checked that all the tests pass and I wasn't able to put together a snippet that showed a problem with your fix. To be continued ...

@aabadie

This comment has been minimized.

Copy link
Contributor

@aabadie aabadie commented Jan 5, 2016

This doesn't feel like the right place to do it but I may be wrong. I double-checked that all the tests pass and I wasn't able to put together a snippet that showed a problem with your fix. To be continued ...

I know but I couldn't find a better place for this. This code seems to be quite racy.

@lesteve

This comment has been minimized.

Copy link
Member

@lesteve lesteve commented Jan 5, 2016

I know but I couldn't find a better place for this. This code seems to be quite racy.

Racy as in subject to race condition I guess ;-). My personal attempt was to set self._iterating to False here and only set it to True if the body of the while loop just below is executed at least once here.

@aabadie

This comment has been minimized.

Copy link
Contributor

@aabadie aabadie commented Jan 5, 2016

Racy as in subject to race condition I guess ;-)

Exactly !

My personal attempt was to set self._iterating to False here and only set it to True if the body of the while loop just below is executed at least once here.

This makes sense and sounds like a better solution.

@aabadie

This comment has been minimized.

Copy link
Contributor

@aabadie aabadie commented Jan 5, 2016

@lesteve, I tried your solution, it works. I'm wondering if this could also solve the issue with appveyor.

@lesteve

This comment has been minimized.

Copy link
Member

@lesteve lesteve commented Jan 5, 2016

@lesteve, I tried your solution, it works. I'm wondering if this could also solve the issue with appveyor.

I'll put together a PR tomorrow for the empty iterator case and try to understand the code better with @ogrisel's help since he is the one who understand it best.

@lesteve

This comment has been minimized.

Copy link
Member

@lesteve lesteve commented Jan 6, 2016

I opened a PR #293. @ogrisel thinks the fix is fine.

@lesteve lesteve closed this in #293 Jan 6, 2016
@joshloyal

This comment has been minimized.

Copy link
Author

@joshloyal joshloyal commented Jan 6, 2016

Great thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.