unordered iteration of AsyncMapResults (+ a couple fixes) #920

Merged
merged 11 commits into from Oct 29, 2011

Projects

None yet

2 participants

@minrk
Member
minrk commented Oct 22, 2011

Feature:

  • AsyncMapResults now support unordered iteration, in case your map is inhomogeneous, and you want to get the quick results first (including new tests).
    Fix:
  • fixed '*' as an ip argument in the kernel/qtconsole and ipcontroller. It was not always getting converted to a connectable interface.

Docs and Examples:

  • added a couple more parallel examples, based on recent discussions (monitoring engine stdout/err, more advanced handling of results as they come in, AsyncMapResult iteration).
  • merged in some cleanup / fixes to the parallel docs from my SciPy 2011 tutorial.
  • moved parallel doc figures into figs subdir
minrk added some commits Oct 22, 2011
@minrk minrk move parallel doc figures into 'figs' subdir
also fix a few broken links after some renames in the examples dir.
ad7b07b
@minrk minrk fix --ip='*' argument in various apps
It wasn't properly converted to localhost/0.0.0.0 everywhere, causing errors on startup.

Fixed in KernelApp, KernelManager (affects QtConsole), and ipcontroller
cde501a
@minrk minrk update parallel docs with some changes from scipy tutorial
also added a couple more parallel examples:

* iopubwatcher for watching stdout/stderr of a cluster
* itermapresult for iterating through AsyncMapResults without waiting for all to complete
* customresults for advanced handling of results not provided by AsyncResult objects
0cd2b7a
@minrk minrk add unordered iteration to AsyncMapResults
only exposed explicitly on LoadBalancedView.map
0d15755
@minrk minrk add '/' separator to keys in SSH engine dict
to prevent confusing with number-ending nodenames in log output.
79a50b9
@minrk minrk add deprecation warning for renamed engine/controller_launcher config
These have been renamed to add _class, which makes it clearer that they
are class names/paths.  This allows 0.11-style specification of the
names to work, but with a warning that the old name is deprecated.
79c1f62
@minrk minrk expand engine/controller_launcher_class helpstring and docs
also fix foo_launcher -> foo_launcher_class in parallel docs

The names for these have been changed in 0.12
ad13d53
@minrk
Member
minrk commented Oct 24, 2011

I also made some further adjustments to docs and helpstrings, based on recent user discussion on-list, including a deprecation warning for the name-change from foo_launcher -> foo_launcher_class.

@minrk minrk add early shutdown detection, and public-ip message to ipcluster/ipen…
…gine

When engines fail to connect, the error message will describe the
most likely cause (not instructing the controller to listen on a public interface).  An similar message is included in ipcluster, but due to restrictions, its trigger is purely time-based.
b85092a
@fperez fperez commented on an outdated diff Oct 28, 2011
IPython/parallel/apps/ipclusterapp.py
@@ -303,13 +321,42 @@ class IPClusterEngines(BaseParallelApplication):
)
return launcher
+ def engines_started_okay(self):
@fperez
fperez Oct 28, 2011 Member

I'd name this just ..._ok for brevity (it also reads better in writing to me, but keep in mind I'm not a native speaker).

@fperez fperez commented on the diff Oct 28, 2011
IPython/parallel/apps/ipcontrollerapp.py
@@ -287,13 +287,15 @@ class IPControllerApp(BaseParallelApplication):
mq = import_item(str(self.mq_class))
hub = self.factory
- # maybe_inproc = 'inproc://monitor' if self.use_threads else self.monitor_url
+ # disambiguate url, in case of *
+ monitor_url = disambiguate_url(hub.monitor_url)
+ # maybe_inproc = 'inproc://monitor' if self.use_threads else monitor_url
@fperez
fperez Oct 28, 2011 Member

was this line meant to be left commented out?

@minrk
minrk Oct 28, 2011 Member

yes, it's an artefact and reminder for using inproc instead of tcp to communicate between the schedulers and the Hub. We can only use it once we bump the pyzmq dependency to 2.1.9 (I think, possibly 2.1.7), which started using Context.instance() in thread devices, which is necessary for inproc support (inproc could also be called in-context, because that is the boundary that matters, not the process).

@fperez fperez and 1 other commented on an outdated diff Oct 28, 2011
IPython/parallel/engine/engine.py
@@ -214,6 +214,14 @@ class EngineFactory(RegistrationFactory):
def abort(self):
self.log.fatal("Registration timed out after %.1f seconds"%self.timeout)
+ if '127' in self.url:
@fperez
fperez Oct 28, 2011 Member

Don't you mean if self.url.startswith('127.')? This will trigger on a perfectly valid IP like 128.32.52.127.

@minrk
minrk Oct 28, 2011 Member

Good call, though it's better to show this message more often than less. I will make the change.

@fperez fperez and 1 other commented on an outdated diff Oct 28, 2011
docs/examples/parallel/itermapresult.py
@@ -0,0 +1,52 @@
+"""Example of iteration through AsyncMapResult, without waiting for all results
@fperez
fperez Oct 28, 2011 Member

I think a slightly more detailed description here of what the example does would be useful. Most people will only read the docstring, so they need a bit more 'meat' in there to decide whether the example explains what they're looking for.

@minrk
minrk Oct 28, 2011 Member

Sure, I'll add a paragraph about heterogeneous workloads, etc.

@fperez
Member
fperez commented Oct 28, 2011

This looks awesome, and I only noticed one small thing that could be a bug. Once that's fixed, this must definitely go in.

If you do it, remember to push a fresh build of the docs, as there's a bunch of great new material here.

Thanks a ton, excellent work!

minrk added some commits Oct 28, 2011
@minrk minrk expand itermapresult docstring 2fad3ac
@minrk minrk cleanup per review by @fperez
* '127' in url -> url.startswith('127.')
* engines_started_okay -> engines_started_ok
fe3786a
@minrk
Member
minrk commented Oct 28, 2011

comments should be addressed

@fperez
Member
fperez commented Oct 28, 2011

@minrk, I got an error in the new test; running with

iptest -vvsx IPython.parallel

I got

======================================================================
ERROR: test_map_ordered (IPython.parallel.tests.test_lbview.TestLoadBalancedView)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/lib/python2.6/site-packages/IPython/parallel/tests/test_lbview.py", line 92, in test_map_ordered
    astheycame = list(amr)
  File "<string>", line 2, in __getattr__
  File "/home/fperez/usr/lib/python2.6/site-packages/IPython/parallel/client/asyncresult.py", line 37, in check_ready
    raise error.TimeoutError("result not ready")
TimeoutError: result not ready

----------------------------------------------------------------------
Ran 61 tests in 33.994s

Thoughts?

@minrk minrk AsyncResult.__getattr__ shouldn't raise TimeoutError
This causes problems for things that use hasattr, e.g. list(ar) checking
for `__length_hint__`.

tests added for getattr/getitem behavior
d1c1aaa
@minrk
Member
minrk commented Oct 29, 2011

Issue is unique to 2.6, and digging reveals it's a bug in the getattr code. In 2.6, list(amr) asks for __length_hint__, whereas 2.7 does not, and the getattr code checked the ready state, raising a TimeoutError. I've fixed it so that it only raises AttributeError or returns metadata. This means that asking for a metadata key by attr (ar.engine_id) will never raise a TimeoutError, it will raise an AttributeError if the metadata is not ready, whereas asking for ar['engine_id'] will raise TimeoutError if it is not available. It made sense to me before, to have getattr and getitem behave the same, but I realize now that raising errors other than AttributeError in getattr is a bad idea.

Tests for getattr and getitem behavior, and explicit test for calling list(amr) have been added to test_asyncresult.

@fperez
Member
fperez commented Oct 29, 2011

Looks good now, tested on 2.6 and 2.7; merging. Thanks for the great work!

@fperez fperez merged commit 854f3d9 into ipython:master Oct 29, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment