Skip to content

Add test for map overlap ordering#3652

Merged
mrocklin merged 2 commits intodask:masterfrom
mrocklin:map-overlap-ordering
Jun 25, 2018
Merged

Add test for map overlap ordering#3652
mrocklin merged 2 commits intodask:masterfrom
mrocklin:map-overlap-ordering

Conversation

@mrocklin
Copy link
Member

@mrocklin mrocklin commented Jun 21, 2018

See #3554

cc @jakirkham

This doesn't resolve the issue in 2d though, and may be just a hack.

  • Tests added / passed
  • Passes flake8 dask

@mrocklin mrocklin force-pushed the map-overlap-ordering branch from af0c116 to 3ddc209 Compare June 21, 2018 22:53
@mrocklin
Copy link
Member Author

cc also @TomAugspurger

@mrocklin
Copy link
Member Author

Actually, as discussed in the original issue, this does seem to have a significant effect in 2d

@TomAugspurger
Copy link
Member

Seems reasonable (as you probably know, this doesn't help out with the ordering issue surfaced in dask-ml).

@jakirkham
Copy link
Member

Interesting, thanks for sharing Matt. Will give this a closer look.

Hmm...does it have negligible impact Tom or does it make things worse?

As you can imagine, we care about optimizing both of these cases as we do use both. :)

@TomAugspurger
Copy link
Member

AFAICT, it has no effect on the other case.

@mrocklin
Copy link
Member Author

From a first principles point of view, this will only improve things, however it may add to the cost of calling order.

@mrocklin
Copy link
Member Author

OK, I tested the increased number of steps through order due to this change by looking at test_gh_3055 and it's something around 5%, which I'm ok with. Merging this.

@mrocklin mrocklin merged commit 59d8c9c into dask:master Jun 25, 2018
@mrocklin mrocklin deleted the map-overlap-ordering branch June 25, 2018 22:40
@jakirkham
Copy link
Member

Thanks @mrocklin :)

convexset added a commit to convexset/dask that referenced this pull request Jul 1, 2018
….com/convexset/dask into fix-tsqr-case-chunk-with-zero-height

* 'fix-tsqr-case-chunk-with-zero-height' of https://github.com/convexset/dask:
  fixed typo in documentation and improved clarity
  Implement .blocks accessor (dask#3689)
  Fix wrong names (dask#3695)
  Adds endpoint and retstep support for linspace (dask#3675)
  Add the @ operator to the delayed objects (dask#3691)
  Align auto chunks to provided chunks, rather than shape (dask#3679)
  Adds quotes to source pip install (dask#3678)
  Prefer end-tasks with low numbers of dependencies when ordering (dask#3588)
  Reimplement argtopk to release the GIL (dask#3610)
  Note `da.pad` can be used with `map_overlap` (dask#3672)
  Allow tasks back onto ordering stack if they have one dependency (dask#3652)
  Fix extra progressbar (dask#3669)
  Break apart uneven array-of-int slicing to separate chunks (dask#3648)
  fix for `dask.array.linalg.tsqr` fails tests (intermittently) with arrays of uncertain dimensions (dask#3662)
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.

3 participants