Use numpy & dask sliding_window_view for rolling #4977

dcherian · 2021-03-01T15:54:22Z

Should merge after dask/dask#7234 is merged

Closes xarray, chunking and rolling operation adds chunking along new dimension (previously worked) #3277, closes DataArray.rolling() does not preserve chunksizes in some cases #2531, closes [WIP] Fix problem with wrong chunksizes when using rolling_window on dask.array #2532, closes interpolate_na with limit argument changes size of chunks #2514
Tests added
Passes pre-commit run --all-files
User visible changes (including notable bug fixes) are documented in whats-new.rst

xarray/tests/test_duck_array_ops.py

xarray/core/rolling.py

xarray/tests/test_variable.py

dcherian · 2021-03-01T17:49:52Z

xarray/tests/test_nputils.py

@@ -29,38 +29,3 @@ def test_vindex():
    vindex[[0, 1], [0, 1], :] = vindex[[0, 1], [0, 1], :]
    vindex[[0, 1], :, [0, 1]] = vindex[[0, 1], :, [0, 1]]
    vindex[:, [0, 1], [0, 1]] = vindex[:, [0, 1], [0, 1]]
-
-


moved to test_variable

dcherian · 2021-03-01T20:54:36Z

sliding_window_view dispatches with array_function so we could support other duck array libraries if they implemented it. cc @keewis

keewis

I didn't do a thorough review, but I have a few comments.

sliding_window_view dispatches with array_function

yes, this is really great.

doc/duckarrays.rst

xarray/core/npcompat.py

xarray/core/variable.py

…indow * upstream/master: add polyval to polyfit see also (pydata#5020) mention map_blocks in the docstring of apply_ufunc (pydata#5011) Switch backend API to v2 (pydata#4989) WIP: add new backend api documentation (pydata#4810) pin netCDF4=1.5.3 in min-all-deps (pydata#4982) fix matplotlib errors for single level discrete colormaps (pydata#4256) Adapt exception handling in CFTimeIndex.__sub__ and __rsub__ (pydata#5006) Update options.py (pydata#5000) Adjust tests to use updated pandas syntax for offsets (pydata#4537) add a combine_attrs parameter to Dataset.merge (pydata#4895) Support for dask.graph_manipulation (pydata#4965) raise on passing axis to Dataset.reduce methods (pydata#4940)

mathause

Looks good. I am still interested to eventually fix #2007. This PR may also make that easier as it now uses Variable.pad.

xarray/core/npcompat.py

xarray/core/dask_array_compat.py

Co-authored-by: Mathias Hauser <[email protected]>

dcherian · 2021-03-13T20:43:43Z

I am still interested to eventually fix #2007. This PR may also make that easier as it now uses Variable.pad.

That's the goal i.e. make it easier to fix #2007 and minimize number of code paths for rolling. This PR reduces the number of places we call pad from 4 to 3 :P. The two bottleneck code paths still do their own padding.

xarray/core/npcompat.py

Co-authored-by: Mathias Hauser <[email protected]>

…indow * upstream/master: Fix regression in decoding large standard calendar times (pydata#5050) Fix sticky sidebar responsiveness on small screens (pydata#5039) Flexible indexes refactoring notes (pydata#4979) add a install xarray step to the upstream-dev CI (pydata#5044) Adds Dataset.query() method, analogous to pandas DataFrame.query() (pydata#4984) run tests on python 3.9 (pydata#5040) Add date attribute to datetime accessor (pydata#4994) 📚 New theme & rearrangement of the docs (pydata#4835) upgrade ci-trigger to the most recent version (pydata#5037) GH5005 fix documentation on open_rasterio (pydata#5021) GHA for automatically canceling previous CI runs (pydata#5025) Implement GroupBy.__getitem__ (pydata#3691) conventions: decode unsigned integers to signed if _Unsigned=false (pydata#4966) Added support for numpy.bool_ (pydata#4986) Add additional str accessor methods for DataArray (pydata#4622)

…nto rolling-sliding-window * 'rolling-sliding-window' of github.com:dcherian/xarray: Update xarray/core/npcompat.py Apply suggestions from code review

dcherian · 2021-03-18T16:31:13Z

The dask PR was merged so this should be ready to go.

mathause · 2021-03-18T16:47:52Z

It looked pretty good when I reviewed it, so 👍

Just quickly skimming the code it did not become entirely clear to me why this had to be after dask/dask#7234? You don't import sliding_window_view from dask. Also you re-define sliding_window_view in dask_array_compat.py - why don't you need this in dask proper? (Not complaining just wondering. Also feel free to not answer the question if you don't have time.)

dcherian · 2021-03-18T17:01:34Z

You don't import sliding_window_view from dask.

haha. fixed!

I was waiting on that PR to see if the reviews would catch any bugs.

andersy005 · 2021-03-18T17:06:46Z

@dcherian, should we test this in upstream-dev CI workflow (via the [test-upstream] keyword)?

dcherian · 2021-03-18T17:10:28Z

thanks @andersy005 !

dcherian added 5 commits March 1, 2021 08:48

Add sliding_window_view functions

76d2c5c

Variable.rolling_window uses Variable.pad & sliding_window_view

979d366

Delete old code

d654942

Update tests

a292c2a

update duckarrays doc

b60617b

dcherian marked this pull request as draft March 1, 2021 15:54

dcherian commented Mar 1, 2021

View reviewed changes

xarray/tests/test_duck_array_ops.py Show resolved Hide resolved

xarray/core/rolling.py Outdated Show resolved Hide resolved

xarray/tests/test_variable.py Show resolved Hide resolved

dcherian added 3 commits March 1, 2021 10:06

convert more nputils tests to Variable tests

5a9f9ef

Undo deletion of bottleneck + dask path

06fb8d7

lint

bfaf076

dcherian commented Mar 1, 2021

View reviewed changes

dcherian added 2 commits March 1, 2021 11:06

vendor ensure minimum chunksize

9f873a0

support older map_overlap signature

f25d16a

more backcompat

bd9bcef

keewis reviewed Mar 1, 2021

View reviewed changes

doc/duckarrays.rst Outdated Show resolved Hide resolved

xarray/core/npcompat.py Outdated Show resolved Hide resolved

xarray/core/variable.py Outdated Show resolved Hide resolved

xarray/core/variable.py Outdated Show resolved Hide resolved

dcherian mentioned this pull request Mar 4, 2021

Bottleneck and dask objects ignore min_periods on rolling #4922

Open

dcherian added 3 commits March 5, 2021 07:07

review feedback

5f86df0

Add some extra tests

61c5e43

mathause reviewed Mar 13, 2021

View reviewed changes

xarray/core/npcompat.py Show resolved Hide resolved

xarray/core/dask_array_compat.py Show resolved Hide resolved

xarray/core/dask_array_compat.py Outdated Show resolved Hide resolved

Apply suggestions from code review

a008aab

Co-authored-by: Mathias Hauser <[email protected]>

mathause reviewed Mar 13, 2021

View reviewed changes

xarray/core/npcompat.py Outdated Show resolved Hide resolved

Update xarray/core/npcompat.py

e41b9a7

Co-authored-by: Mathias Hauser <[email protected]>

dcherian marked this pull request as ready for review March 18, 2021 16:29

dcherian added 3 commits March 18, 2021 10:29

sync with dask PR

cdefb15

Merge branch 'rolling-sliding-window' of github.com:dcherian/xarray i…

f458b7b

…nto rolling-sliding-window * 'rolling-sliding-window' of github.com:dcherian/xarray: Update xarray/core/npcompat.py Apply suggestions from code review

[test-upstream] Actually use dask_array_compat.

39f9e28

dcherian force-pushed the rolling-sliding-window branch from d7c37a7 to 39f9e28 Compare March 18, 2021 17:10

dcherian merged commit fdf024c into pydata:master Mar 26, 2021

dcherian deleted the rolling-sliding-window branch March 26, 2021 19:50

keewis mentioned this pull request Mar 31, 2021

Feature Request: Efficient rolling with strides #3608

Open

Uh oh!

Use numpy & dask sliding_window_view for rolling #4977

Use numpy & dask sliding_window_view for rolling #4977

Uh oh!

Conversation

dcherian commented Mar 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dcherian Mar 1, 2021

Choose a reason for hiding this comment

Uh oh!

dcherian commented Mar 1, 2021

Uh oh!

keewis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mathause left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dcherian commented Mar 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dcherian commented Mar 18, 2021

Uh oh!

mathause commented Mar 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcherian commented Mar 18, 2021

Uh oh!

andersy005 commented Mar 18, 2021

Uh oh!

dcherian commented Mar 18, 2021

Uh oh!

Uh oh!

dcherian commented Mar 1, 2021 •

edited

Loading

dcherian commented Mar 13, 2021 •

edited

Loading

mathause commented Mar 18, 2021 •

edited

Loading