Skip to content

Add cull to remove unnecessary tasks. See issue #2.#10

Merged
mrocklin merged 2 commits intodask:masterfrom
eriknw:cull
Jan 19, 2015
Merged

Add cull to remove unnecessary tasks. See issue #2.#10
mrocklin merged 2 commits intodask:masterfrom
eriknw:cull

Conversation

@eriknw
Copy link
Member

@eriknw eriknw commented Jan 19, 2015

For now, I just put this in dask.core.

For posterity, the following is a more concise solution, but is slower:

from toolz import concat, keyfilter

def cull(dsk, keys):
    if not isinstance(keys, list):
        keys = [keys]
    seen = set()
    cur = set(keys)
    while cur:
        seen.update(cur)
        cur = set(concat(get_dependencies(dsk, x) for x in cur))
        cur.difference_update(seen)
    return keyfilter(seen.__contains__, dsk)

cpcloud
cpcloud reviewed Jan 19, 2015
View reviewed changes
dask/core.py
Copy link
Contributor

@cpcloud cpcloud Jan 19, 2015

Choose a reason for hiding this comment

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

can keys ever be a tuple?

Copy link
Contributor

@cpcloud cpcloud Jan 19, 2015

Choose a reason for hiding this comment

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

and does that matter?

Copy link
Member Author

@eriknw eriknw Jan 19, 2015

Choose a reason for hiding this comment

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

I believe there is already a precedence to only use lists to specify multiple keys. After all, a tuple is hashable and may be used as a key.

Copy link
Member

@mrocklin mrocklin Jan 19, 2015

Choose a reason for hiding this comment

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

Correct. tuple is already overloaded to possibly mean a key or a task. We use lists as collections because they couldn't possibly mean a single key and so are unambiguous. Probably I should mention these things more clearly in the docs

mrocklin
mrocklin reviewed Jan 19, 2015
View reviewed changes
dask/core.py
Copy link
Member

@mrocklin mrocklin Jan 19, 2015

Choose a reason for hiding this comment

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

Whoops! Thanks.

mrocklin added a commit that referenced this pull request Jan 19, 2015
@mrocklin
Merge pull request #10 from eriknw/cull
f8f7215 
Add `cull` to remove unnecessary tasks.  See issue #2.
@mrocklin mrocklin merged commit f8f7215 into dask:master Jan 19, 2015
@eriknw eriknw mentioned this pull request Jan 21, 2015
Closed
mrocklin referenced this pull request in mrocklin/dask Mar 28, 2019
@TomAugspurger @mrocklin
Persist X in K-Means (#10)
1ae6be8 
* Persist X in K-Means

* Strip output
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@eriknw @mrocklin @cpcloud