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

get_dict() error #3646

Closed
remram44 opened this issue Jul 15, 2013 · 11 comments · Fixed by #3649
Closed

get_dict() error #3646

remram44 opened this issue Jul 15, 2013 · 11 comments · Fixed by #3649
Assignees
Labels
Milestone

Comments

@remram44
Copy link
Contributor

get_dict() doesn't return the expected {engine_id: result} dictionary when the AsyncResult is associated with a single engine.

from IPython.parallel import Client

rc = Client()
rc.ids # [0]

r = rc.load_balanced_view().apply_async(eval, '(1, 2, 3)')

r.get() # (1, 2, 3)
r.get_dict() # {0: 1}
# {0: (1, 2, 3)} was expected
@ivanov
Copy link
Member

ivanov commented Jul 15, 2013

can you add more details? this is pretty cryptic

@remram44
Copy link
Contributor Author

Wow, I didn't expect you to read this so fast! Here are the details.

@ivanov
Copy link
Member

ivanov commented Jul 15, 2013

pinging @minrk since this involves parallel

minrk added a commit to minrk/ipython that referenced this issue Jul 15, 2013
avoids issue with LoadBalancedView, client[0], etc.

Also cleanup the multiple-key test while I'm there,
because it was gross.

closes ipython#3646
@minrk
Copy link
Member

minrk commented Jul 15, 2013

get_dict doesn't make much sense outside of multi-engine DirectView, but it's an easy fix. See #3649 for fix and tests.

@ghost ghost assigned minrk Jul 15, 2013
@remram44
Copy link
Contributor Author

I disagree, of course. Having to check how many engines are in the view before calling is extremely annoying, and also makes for a race condition.

Thanks for the fix!

@minrk
Copy link
Member

minrk commented Jul 15, 2013

How could it possibly make for a race condition? Single-engine-ness cannot change without explicit user intervention, and generally doesn't (and shouldn't) happen without creating a new View.

Can I ask in what context you ran into this issue? I'm always interesting in how people are using IPython.parallel.

@remram44
Copy link
Contributor Author

Single-engine-ness cannot change without explicit user intervention, and generally doesn't (and shouldn't) happen without creating a new View.

DirectView is fine, but a LoadBalancedView targeting all engines will make use of new engines without the need to get recreate it. This means that engines can register (or die) before you submit the request. But anyway.


I'm using IPython inside an application (VisTrails, although this particular work is not public yet) that provides a GUI for using IPython; the particular piece of code that triggered the issue is one that gets some basic info from the engines:

with client.direct_view() as dview:
    engines = dview.apply_async(
            eval,
            '(os.getpid(), platform.system(), socket.getfqdn())'
    ).get_dict()
    print "engines:\n\tid\tsystem\tPID\tnode FQDN"
    for ip_id, (pid, system, fqdn) in engines:
        print "\t%d\t%s\t%d\t%s" % (ip_id, system, pid, fqdn)

I'm also using a wrapper for Client to provide callbacks for AsyncResults, and thread-safety; I think we discussed that before?

@minrk
Copy link
Member

minrk commented Jul 15, 2013

DirectView is fine, but a LoadBalancedView targeting all engines will make use of new engines without the need to get recreate it.

A load-balanced view is always a single-engine view in the way that matters for get_dict. Single-engine-ness only means that when you call AsyncResult.get(), you will get the actual result rather than a list of length one. This is always true of LoadBalancedViews and DirectViews created via engine = client[N], and not true otherwise.

I don't see how the code you pasted could be what triggered the issue, because direct_view() will not create single-engine views unless you pass the argument targets=<integer>. If it did, there is another bug I need to track down.

minrk added a commit that referenced this issue Jul 16, 2013
fix AsyncResult.get_dict for single result

and add tests for single-result and invalid input (multiple results on one engine).

closes #3646
@remram44
Copy link
Contributor Author

direct_view() will not create single-engine views unless you pass the argument targets=. If it did, there is another bug I need to track down.

Passing targets='all' will get you such a view if there is only one engine. I can't reproduce the bug through slicing (i.e. rc[:] works but rc.direct_view() will exhibit the problem).

@minrk
Copy link
Member

minrk commented Jul 16, 2013

Passing targets='all' will get you such a view if there is only one engine.

Thanks, this is most certainly a bug. Will fix shortly.

@minrk
Copy link
Member

minrk commented Jul 16, 2013

#3654 should fix the issue of targets='all' behaving like targets=client.ids[0] when only one engine is present.

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
avoids issue with LoadBalancedView, client[0], etc.

Also cleanup the multiple-key test while I'm there,
because it was gross.

closes ipython#3646
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
fix AsyncResult.get_dict for single result

and add tests for single-result and invalid input (multiple results on one engine).

closes ipython#3646
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants