Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Get progress from xarray / dask tasks #290

Merged
merged 13 commits into from
Jul 12, 2017
Merged

Get progress from xarray / dask tasks #290

merged 13 commits into from
Jul 12, 2017

Conversation

mzuehlke
Copy link
Collaborator

@mzuehlke mzuehlke commented Jul 11, 2017

This adds a dask.Callback handler that translates the callbacks emitted from the dask.scheduler into progress for the cate.Monitor and allows to cancel the tasks.

I've used the new DaskMonitor in the subset and coregister op to show the usage and test its usability.

There are some more ops that would profit from using the DaskMonitor. Whenever a xarray function invocation results in a dask get or compute function call.

fixes #282
Closes #288

@mzuehlke mzuehlke requested review from forman and JanisGailis July 11, 2017 08:52
@codecov-io
Copy link

codecov-io commented Jul 11, 2017

Codecov Report

Merging #290 into master will increase coverage by 0.11%.
The diff coverage is 83.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #290      +/-   ##
==========================================
+ Coverage   71.63%   71.74%   +0.11%     
==========================================
  Files          69       69              
  Lines        9870     9913      +43     
==========================================
+ Hits         7070     7112      +42     
- Misses       2800     2801       +1
Impacted Files Coverage Δ
cate/ds/esa_cci_odp.py 52.07% <ø> (+0.1%) ⬆️
cate/core/op.py 95.23% <ø> (ø) ⬆️
cate/ops/utility.py 80.39% <ø> (+3.03%) ⬆️
cate/util/web/jsonrpcmonitor.py 27.65% <0%> (-0.61%) ⬇️
cate/ops/outliers.py 100% <100%> (ø) ⬆️
cate/util/__init__.py 100% <100%> (ø) ⬆️
cate/ops/coregistration.py 97.97% <100%> (+0.15%) ⬆️
cate/ds/esa_cci_ftp.py 45.59% <25%> (+0.28%) ⬆️
cate/ops/io.py 87.3% <50%> (-0.39%) ⬇️
cate/util/web/jsonrpchandler.py 15.78% <50%> (ø) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 714523c...a27b6b4. Read the comment docs.

@JanisGailis
Copy link
Member

This is very exciting! I'll take a look and test how it works!

Copy link
Member

@JanisGailis JanisGailis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked out the branch and tried running co-registration from the UC9 notebook with a monitor. Works like charm! Great to have this!

One note though - maybe you can check out doc/source/development_guide/dg-operation-development.rst and add a sentence or two and a quick example about this in the chapter on using Cate monitor?

Even if you don't do this I still want to have this merged :)

@forman
Copy link
Member

forman commented Jul 11, 2017

What about:

monitor = parent_monitor.child(1)
with monitor.observing("resample time slice"):
    result = resampling.resample_2d(np.ma.masked_invalid(arr_slice.values),
    ...

@forman
Copy link
Member

forman commented Jul 11, 2017

Regarding the

if monitor.is_cancelled():
    raise InterruptedError

... would be nice if Monitor impls could raise this on their own when a method is called but they are already cancelled. Very useful in Monitor.progress().

@forman
Copy link
Member

forman commented Jul 11, 2017

As just discussed: Cancellation(Exception) to distinguish between interruption errors like timeouts, kill -9, etc and true cancellation by users.

mzuehlke added 6 commits July 11, 2017 14:18
* monitor.progress() automatically checks for cancellation
* `Cancellation(Excpetion)` is rasie instead of the generic `ÌnteruptedError`
* use a context manager function on `Monitor` to create an _observing_ monitor
@mzuehlke mzuehlke merged commit 672ddf8 into master Jul 12, 2017
@mzuehlke mzuehlke deleted the 282-mz-dask_progress branch July 12, 2017 11:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement monitor in coregistration Using Cate monitor with deferred processing
4 participants