Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rolling operation with dask and bottleneck #2942

Closed
wants to merge 2 commits into from

Conversation

fujiisoup
Copy link
Member

Fix for #2940
It looks that there was a bug in the previous logic, but I am not sure why it was working...

@fujiisoup fujiisoup changed the title Fix: 2940 Fix rolling operation with dask and bottleneck May 6, 2019
@shoyer
Copy link
Member

shoyer commented May 7, 2019

@fujiisoup, thanks for looking into this! Unfortunately these tests still seem to still be failing on Travis.

@dcherian dcherian mentioned this pull request May 21, 2019
15 tasks
@max-sixty
Copy link
Collaborator

max-sixty commented Jun 15, 2019

@fujiisoup I mistakenly removed the dask-master tests (I moved the dask test from the test matrix to the ignored failures, rather than adding it to ignored failures)

Here's a tiny patch which restores it, if you want to add this to your PR:

EDIT: moved to separate PR

diff --git a/.travis.yml b/.travis.yml
index 913c5e1c..8249b134 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -19,12 +19,12 @@ matrix:
   - env: CONDA_ENV=py36-pandas-dev
   - env: CONDA_ENV=py36-rasterio
   - env: CONDA_ENV=py36-zarr-dev
+  - env: CONDA_ENV=py36-dask-dev
   - env: CONDA_ENV=docs
   - env: CONDA_ENV=lint
   - env: CONDA_ENV=py36-hypothesis
 
   allow_failures:
-  - env: CONDA_ENV=py36-dask-dev
   - env:
     - CONDA_ENV=py36
     - EXTRA_FLAGS="--run-flaky --run-network-tests"

@zbarry
Copy link

zbarry commented Jun 18, 2019

Hey just wanted to chime in and say it appears that commit fujiisoup@098daf3 is still losing chunking for me as far as I can tell when running it with dask distributed / dask jobqueue. I can do some extended testing & look further into this if people have some suggestions for how to go about it.

@fujiisoup

@shoyer
Copy link
Member

shoyer commented Jun 20, 2019

I did a little more looking into this, and I unfortunately I think the necessary fix is a little more involved. For now, I think we should disable rolling() with dask arrays to avoid computing incorrect results: #2940 (comment)

@fujiisoup
Copy link
Member Author

Hi. Sorry for my slow responce...

lf I understood correctly, it happens with dask and bottleneck.
So, probably we can skip using bottleneck if the backend array is dask array?

@shoyer
Copy link
Member

shoyer commented Jun 23, 2019

@fujiisoup yes, indeed, that's a much better temporary fix! See #3040.

@shoyer
Copy link
Member

shoyer commented Jun 30, 2019

This is subsumed by #3040

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.

test_rolling_wrapped_dask is failing with dask-master
4 participants