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

allow map / parallel function for single-engine views #1613

Merged
merged 1 commit into from Apr 16, 2012

Conversation

minrk
Copy link
Member

@minrk minrk commented Apr 16, 2012

it's a bit of a weird thing to do, but I don't see a significant reason not to allow it. Map/parallel functions are unambiguous in what they should return (a list of length one), as opposed to apply, where there is a distinction between:

e0a = e[0] # apply returns single result

e0b = e[:1] # apply returns list of length 1

An alternative would be to just give a better error message when trying map/parallel functions on single-engine views.

@fperez
Copy link
Member

fperez commented Apr 16, 2012

Mmh, you're giving me pause now... I like the simplicity of 'doing what the user wanted' for new/naive users. But at the same time, I know that sometimes making libraries 'too smart' ends up causing trouble down the road. I fail to foresee a problem with this simple enhancement, but the fact that tagets gets converted to a list, while the .apply semantics still distinguish between the two cases, makes me think that keeping the old behavior albeit with an easier to understand error message might be better... Something like "only views created with a list of engines can use the map* methods, use client[n:n+1] syntax if you need a single-engine view".

Thoughts?

@minrk
Copy link
Member Author

minrk commented Apr 16, 2012

I'm pretty close to 50/50 on this, and implemented it both ways, switching back and forth a few times before issuing the PR, but I came down on the side of not surprising users with artificial error messages when user intent is obvious, makes sense, and is easily supported.

For map specifically, there is exactly no ambiguity about what should be returned: It should be a list, no matter how/where it is called. In this case, e0.map(f, iterable) is exactly identical to e0.apply(map, f, iterable). The same code is submitted in the same number of messages (1).

This is substantially different from apply, which only means 'call this function remotely', but with lots of overloaded information packed into properties of the View itself (where/when, load-balance/mux, etc.) affecting what is actually returned.

I think another point is that map is not a necessarily parallel call - it's a builtin function that makes sense locally, so I think it makes sense to be able to call it remotely on a single engine. A clearer way to do this would be the .apply(map, f...) manner above, but I don't see a significant reason to prevent this method from working as well.

Note that targets doesn't get 'converted to a list' in any preserved way, it's just that I use len and enumerate on it, so it was simpler to wrap it in a list once than special case those two calls. The targets are not preserved beyond the submission of the tasks, and they are only used to determine the initial partition.

Also, better than client[n:n+1] is, I think, client[[n]] if we go that route. It's clearer (to me) that it is just a list containing the ID.

@fperez
Copy link
Member

fperez commented Apr 16, 2012

OK, let's go for user friendliness here. Merging as-is, thanks!!

fperez added a commit that referenced this pull request Apr 16, 2012
allow map / parallel function for single-engine views
@fperez fperez merged commit 2464446 into ipython:master Apr 16, 2012
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
allow map / parallel function for single-engine views
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants