Skip to content

Fix io_deps bug in rewrite_blockwise#6989

Closed
rjzamora wants to merge 2 commits intodask:masterfrom
rjzamora:fix-rewrite_blockwise
Closed

Fix io_deps bug in rewrite_blockwise#6989
rjzamora wants to merge 2 commits intodask:masterfrom
rjzamora:fix-rewrite_blockwise

Conversation

@rjzamora
Copy link
Member

Addresses xarray#4703 (which is alse described in #6931 )

The current rewrite_blockwise code is updating a list of IO-dependencies (io_deps) within a loop that can be escaped before all dependencies are collected. This is causing CI errors in xarray. This PR moves the io_deps update to a simple/dedicated loop over all input layers.

TODO:

  • come up with a test

@rjzamora
Copy link
Member Author

cc @dcherian - I'm hoping this is the only fix needed for the xarray CI failures. Please do let me know if you already have a simple dask.array test handy :)

@dcherian
Copy link
Collaborator

I confirmed that xarray's CI passes on this branch locally.

I could not create a pure dask.array test, or even a pure xarray test!

@ian-r-rose
Copy link
Collaborator

I could not create a pure dask.array test, or even a pure xarray test!

I also tried to create a test, and found it really challenging! The xarray test has some very subtle things going on with null checks in a string array that I wasn't able to untangle.

@dcherian
Copy link
Collaborator

Yes it's breaking on

bool(
    (
        (arr1 == arr2) | (dask.array.isnull(arr1) & dask.array.isnull(arr2))
    ).any()
)

arr1 and arr2 are dask arrays created using from_array with either str or int64 dtypes, and sometimes there is a slice involved. In one case arr1 had unknown shape and chunksize.

rjzamora added a commit to rjzamora/dask that referenced this pull request Jan 7, 2021
@rjzamora
Copy link
Member Author

rjzamora commented Jan 7, 2021

Closing in favor of #7042

@rjzamora rjzamora closed this Jan 7, 2021
@rjzamora rjzamora deleted the fix-rewrite_blockwise branch January 8, 2021 14:47
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