From b79155862e154db45dff3b526c2f9e8ef303808f Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 23 Oct 2021 19:13:51 -0700 Subject: [PATCH 1/5] Check jupyter nbs with black in pre-commit (#5891) --- .pre-commit-config.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 51eed6a6fc9..27f93c8e578 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -16,6 +16,7 @@ repos: rev: 21.9b0 hooks: - id: black + - id: black-jupyter - repo: https://github.com/keewis/blackdoc rev: v0.3.4 hooks: From ea2886136dec7047186d5a73380d50130a7b5241 Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Sun, 24 Oct 2021 11:54:28 +0200 Subject: [PATCH 2/5] Single matplotlib import (#5794) --- asv_bench/benchmarks/import_xarray.py | 9 +++++++ xarray/plot/dataset_plot.py | 12 ++++------ xarray/plot/facetgrid.py | 10 ++------ xarray/plot/plot.py | 8 +------ xarray/plot/utils.py | 34 ++++++++++++--------------- 5 files changed, 31 insertions(+), 42 deletions(-) create mode 100644 asv_bench/benchmarks/import_xarray.py diff --git a/asv_bench/benchmarks/import_xarray.py b/asv_bench/benchmarks/import_xarray.py new file mode 100644 index 00000000000..94652e3b82a --- /dev/null +++ b/asv_bench/benchmarks/import_xarray.py @@ -0,0 +1,9 @@ +class ImportXarray: + def setup(self, *args, **kwargs): + def import_xr(): + import xarray # noqa: F401 + + self._import_xr = import_xr + + def time_import_xarray(self): + self._import_xr() diff --git a/xarray/plot/dataset_plot.py b/xarray/plot/dataset_plot.py index c1aedd570bc..7288a368e47 100644 --- a/xarray/plot/dataset_plot.py +++ b/xarray/plot/dataset_plot.py @@ -12,6 +12,7 @@ _process_cmap_cbar_kwargs, get_axis, label_from_attrs, + plt, ) # copied from seaborn @@ -134,8 +135,7 @@ def _infer_scatter_data(ds, x, y, hue, markersize, size_norm, size_mapping=None) # copied from seaborn def _parse_size(data, norm): - - import matplotlib as mpl + mpl = plt.matplotlib if data is None: return None @@ -544,8 +544,6 @@ def quiver(ds, x, y, ax, u, v, **kwargs): Wraps :py:func:`matplotlib:matplotlib.pyplot.quiver`. """ - import matplotlib as mpl - if x is None or y is None or u is None or v is None: raise ValueError("Must specify x, y, u, v for quiver plots.") @@ -560,7 +558,7 @@ def quiver(ds, x, y, ax, u, v, **kwargs): # TODO: Fix this by always returning a norm with vmin, vmax in cmap_params if not cmap_params["norm"]: - cmap_params["norm"] = mpl.colors.Normalize( + cmap_params["norm"] = plt.Normalize( cmap_params.pop("vmin"), cmap_params.pop("vmax") ) @@ -576,8 +574,6 @@ def streamplot(ds, x, y, ax, u, v, **kwargs): Wraps :py:func:`matplotlib:matplotlib.pyplot.streamplot`. """ - import matplotlib as mpl - if x is None or y is None or u is None or v is None: raise ValueError("Must specify x, y, u, v for streamplot plots.") @@ -613,7 +609,7 @@ def streamplot(ds, x, y, ax, u, v, **kwargs): # TODO: Fix this by always returning a norm with vmin, vmax in cmap_params if not cmap_params["norm"]: - cmap_params["norm"] = mpl.colors.Normalize( + cmap_params["norm"] = plt.Normalize( cmap_params.pop("vmin"), cmap_params.pop("vmax") ) diff --git a/xarray/plot/facetgrid.py b/xarray/plot/facetgrid.py index 28dd82e76f5..b384dea0571 100644 --- a/xarray/plot/facetgrid.py +++ b/xarray/plot/facetgrid.py @@ -9,8 +9,8 @@ _get_nice_quiver_magnitude, _infer_xy_labels, _process_cmap_cbar_kwargs, - import_matplotlib_pyplot, label_from_attrs, + plt, ) # Overrides axes.labelsize, xtick.major.size, ytick.major.size @@ -116,8 +116,6 @@ def __init__( """ - plt = import_matplotlib_pyplot() - # Handle corner case of nonunique coordinates rep_col = col is not None and not data[col].to_index().is_unique rep_row = row is not None and not data[row].to_index().is_unique @@ -519,10 +517,8 @@ def set_titles(self, template="{coord} = {value}", maxchar=30, size=None, **kwar self: FacetGrid object """ - import matplotlib as mpl - if size is None: - size = mpl.rcParams["axes.labelsize"] + size = plt.rcParams["axes.labelsize"] nicetitle = functools.partial(_nicetitle, maxchar=maxchar, template=template) @@ -619,8 +615,6 @@ def map(self, func, *args, **kwargs): self : FacetGrid object """ - plt = import_matplotlib_pyplot() - for ax, namedict in zip(self.axes.flat, self.name_dicts.flat): if namedict is not None: data = self.data.loc[namedict] diff --git a/xarray/plot/plot.py b/xarray/plot/plot.py index dffdde25db4..1e1e59e2f71 100644 --- a/xarray/plot/plot.py +++ b/xarray/plot/plot.py @@ -29,9 +29,9 @@ _resolve_intervals_2dplot, _update_axes, get_axis, - import_matplotlib_pyplot, label_from_attrs, legend_elements, + plt, ) # copied from seaborn @@ -83,8 +83,6 @@ def _parse_size(data, norm, width): If the data is categorical, normalize it to numbers. """ - plt = import_matplotlib_pyplot() - if data is None: return None @@ -682,8 +680,6 @@ def scatter( **kwargs : optional Additional keyword arguments to matplotlib """ - plt = import_matplotlib_pyplot() - # Handle facetgrids first if row or col: allargs = locals().copy() @@ -1111,8 +1107,6 @@ def newplotfunc( allargs["plotfunc"] = globals()[plotfunc.__name__] return _easy_facetgrid(darray, kind="dataarray", **allargs) - plt = import_matplotlib_pyplot() - if ( plotfunc.__name__ == "surface" and not kwargs.get("_is_facetgrid", False) diff --git a/xarray/plot/utils.py b/xarray/plot/utils.py index 594f2e5360e..6fbbe9d4bca 100644 --- a/xarray/plot/utils.py +++ b/xarray/plot/utils.py @@ -47,6 +47,12 @@ def import_matplotlib_pyplot(): return plt +try: + plt = import_matplotlib_pyplot() +except ImportError: + plt = None + + def _determine_extend(calc_data, vmin, vmax): extend_min = calc_data.min() < vmin extend_max = calc_data.max() > vmax @@ -64,7 +70,7 @@ def _build_discrete_cmap(cmap, levels, extend, filled): """ Build a discrete colormap and normalization of the data. """ - import matplotlib as mpl + mpl = plt.matplotlib if len(levels) == 1: levels = [levels[0], levels[0]] @@ -115,8 +121,7 @@ def _build_discrete_cmap(cmap, levels, extend, filled): def _color_palette(cmap, n_colors): - import matplotlib.pyplot as plt - from matplotlib.colors import ListedColormap + ListedColormap = plt.matplotlib.colors.ListedColormap colors_i = np.linspace(0, 1.0, n_colors) if isinstance(cmap, (list, tuple)): @@ -177,7 +182,7 @@ def _determine_cmap_params( cmap_params : dict Use depends on the type of the plotting function """ - import matplotlib as mpl + mpl = plt.matplotlib if isinstance(levels, Iterable): levels = sorted(levels) @@ -285,13 +290,13 @@ def _determine_cmap_params( levels = np.asarray([(vmin + vmax) / 2]) else: # N in MaxNLocator refers to bins, not ticks - ticker = mpl.ticker.MaxNLocator(levels - 1) + ticker = plt.MaxNLocator(levels - 1) levels = ticker.tick_values(vmin, vmax) vmin, vmax = levels[0], levels[-1] # GH3734 if vmin == vmax: - vmin, vmax = mpl.ticker.LinearLocator(2).tick_values(vmin, vmax) + vmin, vmax = plt.LinearLocator(2).tick_values(vmin, vmax) if extend is None: extend = _determine_extend(calc_data, vmin, vmax) @@ -421,10 +426,7 @@ def _assert_valid_xy(darray, xy, name): def get_axis(figsize=None, size=None, aspect=None, ax=None, **kwargs): - try: - import matplotlib as mpl - import matplotlib.pyplot as plt - except ImportError: + if plt is None: raise ImportError("matplotlib is required for plot.utils.get_axis") if figsize is not None: @@ -437,7 +439,7 @@ def get_axis(figsize=None, size=None, aspect=None, ax=None, **kwargs): if ax is not None: raise ValueError("cannot provide both `size` and `ax` arguments") if aspect is None: - width, height = mpl.rcParams["figure.figsize"] + width, height = plt.rcParams["figure.figsize"] aspect = width / height figsize = (size * aspect, size) _, ax = plt.subplots(figsize=figsize) @@ -454,9 +456,6 @@ def get_axis(figsize=None, size=None, aspect=None, ax=None, **kwargs): def _maybe_gca(**kwargs): - - import matplotlib.pyplot as plt - # can call gcf unconditionally: either it exists or would be created by plt.axes f = plt.gcf() @@ -912,9 +911,7 @@ def _process_cmap_cbar_kwargs( def _get_nice_quiver_magnitude(u, v): - import matplotlib as mpl - - ticker = mpl.ticker.MaxNLocator(3) + ticker = plt.MaxNLocator(3) mean = np.mean(np.hypot(u.to_numpy(), v.to_numpy())) magnitude = ticker.tick_values(0, mean)[-2] return magnitude @@ -989,7 +986,7 @@ def legend_elements( """ import warnings - import matplotlib as mpl + mpl = plt.matplotlib mlines = mpl.lines @@ -1126,7 +1123,6 @@ def _legend_add_subtitle(handles, labels, text, func): def _adjust_legend_subtitles(legend): """Make invisible-handle "subtitles" entries look more like titles.""" - plt = import_matplotlib_pyplot() # Legend title not in rcParams until 3.0 font_size = plt.rcParams.get("legend.title_fontsize", None) From 69dec51cfca065f2abdc9933c938c8c03e694184 Mon Sep 17 00:00:00 2001 From: Spencer Clark Date: Sun, 24 Oct 2021 05:55:33 -0400 Subject: [PATCH 3/5] Remove use of deprecated `kind` argument in `CFTimeIndex` tests (#5723) --- doc/whats-new.rst | 3 ++ xarray/tests/test_cftimeindex.py | 58 ++++++++++++++++++++++---------- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 1ade92f8588..ae66f9d33ae 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -106,6 +106,9 @@ Internal Changes By `Jimmy Westling `_. - Use isort's `float_to_top` config. (:pull:`5695`). By `Maximilian Roos `_. +- Remove use of the deprecated ``kind`` argument in + :py:meth:`pandas.Index.get_slice_bound` inside :py:class:`xarray.CFTimeIndex` + tests (:pull:`5723`). By `Spencer Clark `_. - Refactor `xarray.core.duck_array_ops` to no longer special-case dispatching to dask versions of functions when acting on dask arrays, instead relying numpy and dask's adherence to NEP-18 to dispatch automatically. (:pull:`5571`) diff --git a/xarray/tests/test_cftimeindex.py b/xarray/tests/test_cftimeindex.py index 725b5efee75..619fb0acdc4 100644 --- a/xarray/tests/test_cftimeindex.py +++ b/xarray/tests/test_cftimeindex.py @@ -1,4 +1,5 @@ from datetime import timedelta +from distutils.version import LooseVersion from textwrap import dedent import numpy as np @@ -345,65 +346,86 @@ def test_get_loc(date_type, index): @requires_cftime -@pytest.mark.parametrize("kind", ["loc", "getitem"]) -def test_get_slice_bound(date_type, index, kind): - result = index.get_slice_bound("0001", "left", kind) +def test_get_slice_bound(date_type, index): + # The kind argument is required in earlier versions of pandas even though it + # is not used by CFTimeIndex. This logic can be removed once our minimum + # version of pandas is at least 1.3. + if LooseVersion(pd.__version__) < LooseVersion("1.3"): + kind_args = ("getitem",) + else: + kind_args = () + + result = index.get_slice_bound("0001", "left", *kind_args) expected = 0 assert result == expected - result = index.get_slice_bound("0001", "right", kind) + result = index.get_slice_bound("0001", "right", *kind_args) expected = 2 assert result == expected - result = index.get_slice_bound(date_type(1, 3, 1), "left", kind) + result = index.get_slice_bound(date_type(1, 3, 1), "left", *kind_args) expected = 2 assert result == expected - result = index.get_slice_bound(date_type(1, 3, 1), "right", kind) + result = index.get_slice_bound(date_type(1, 3, 1), "right", *kind_args) expected = 2 assert result == expected @requires_cftime -@pytest.mark.parametrize("kind", ["loc", "getitem"]) -def test_get_slice_bound_decreasing_index(date_type, monotonic_decreasing_index, kind): - result = monotonic_decreasing_index.get_slice_bound("0001", "left", kind) +def test_get_slice_bound_decreasing_index(date_type, monotonic_decreasing_index): + # The kind argument is required in earlier versions of pandas even though it + # is not used by CFTimeIndex. This logic can be removed once our minimum + # version of pandas is at least 1.3. + if LooseVersion(pd.__version__) < LooseVersion("1.3"): + kind_args = ("getitem",) + else: + kind_args = () + + result = monotonic_decreasing_index.get_slice_bound("0001", "left", *kind_args) expected = 2 assert result == expected - result = monotonic_decreasing_index.get_slice_bound("0001", "right", kind) + result = monotonic_decreasing_index.get_slice_bound("0001", "right", *kind_args) expected = 4 assert result == expected result = monotonic_decreasing_index.get_slice_bound( - date_type(1, 3, 1), "left", kind + date_type(1, 3, 1), "left", *kind_args ) expected = 2 assert result == expected result = monotonic_decreasing_index.get_slice_bound( - date_type(1, 3, 1), "right", kind + date_type(1, 3, 1), "right", *kind_args ) expected = 2 assert result == expected @requires_cftime -@pytest.mark.parametrize("kind", ["loc", "getitem"]) -def test_get_slice_bound_length_one_index(date_type, length_one_index, kind): - result = length_one_index.get_slice_bound("0001", "left", kind) +def test_get_slice_bound_length_one_index(date_type, length_one_index): + # The kind argument is required in earlier versions of pandas even though it + # is not used by CFTimeIndex. This logic can be removed once our minimum + # version of pandas is at least 1.3. + if LooseVersion(pd.__version__) <= LooseVersion("1.3"): + kind_args = ("getitem",) + else: + kind_args = () + + result = length_one_index.get_slice_bound("0001", "left", *kind_args) expected = 0 assert result == expected - result = length_one_index.get_slice_bound("0001", "right", kind) + result = length_one_index.get_slice_bound("0001", "right", *kind_args) expected = 1 assert result == expected - result = length_one_index.get_slice_bound(date_type(1, 3, 1), "left", kind) + result = length_one_index.get_slice_bound(date_type(1, 3, 1), "left", *kind_args) expected = 1 assert result == expected - result = length_one_index.get_slice_bound(date_type(1, 3, 1), "right", kind) + result = length_one_index.get_slice_bound(date_type(1, 3, 1), "right", *kind_args) expected = 1 assert result == expected From 26e2e61e27a04a78f4887ecdc8166715a9b99c1a Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Sun, 24 Oct 2021 12:08:01 +0200 Subject: [PATCH 4/5] Add asv benchmark jobs to CI (#5796) Also fix many benchmarks. --- .github/workflows/benchmarks.yml | 77 ++++++++++++++ asv_bench/benchmarks/README_CI.md | 122 ++++++++++++++++++++++ asv_bench/benchmarks/__init__.py | 19 ++++ asv_bench/benchmarks/combine.py | 2 +- asv_bench/benchmarks/dataarray_missing.py | 120 +++++++++++---------- asv_bench/benchmarks/dataset_io.py | 17 ++- asv_bench/benchmarks/indexing.py | 36 +++---- asv_bench/benchmarks/interp.py | 14 +-- asv_bench/benchmarks/pandas.py | 6 +- asv_bench/benchmarks/reindexing.py | 22 ++-- asv_bench/benchmarks/repr.py | 4 +- asv_bench/benchmarks/rolling.py | 20 ++-- asv_bench/benchmarks/unstacking.py | 4 +- doc/whats-new.rst | 2 + 14 files changed, 348 insertions(+), 117 deletions(-) create mode 100644 .github/workflows/benchmarks.yml create mode 100644 asv_bench/benchmarks/README_CI.md diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml new file mode 100644 index 00000000000..7c3cb114201 --- /dev/null +++ b/.github/workflows/benchmarks.yml @@ -0,0 +1,77 @@ +name: Benchmark + +on: + pull_request: + types: [opened, reopened, synchronize, labeled] + workflow_dispatch: + +jobs: + benchmark: + if: | + ${{ contains( github.event.pull_request.labels.*.name, 'run-benchmark') + && github.event_name == 'pull_request' + || github.event_name == 'workflow_dispatch' }} + name: Linux + runs-on: ubuntu-20.04 + env: + ASV_DIR: "./asv_bench" + + steps: + # We need the full repo to avoid this issue + # https://github.com/actions/checkout/issues/23 + - uses: actions/checkout@v2 + with: + fetch-depth: 0 + + - name: Setup Miniconda + uses: conda-incubator/setup-miniconda@v2 + with: + # installer-url: https://github.com/conda-forge/miniforge/releases/latest/download/Mambaforge-Linux-x86_64.sh + installer-url: https://github.com/conda-forge/miniforge/releases/latest/download/Miniforge3-Linux-x86_64.sh + + - name: Setup some dependencies + shell: bash -l {0} + run: | + pip install asv + sudo apt-get update -y + + - name: Run benchmarks + shell: bash -l {0} + id: benchmark + env: + OPENBLAS_NUM_THREADS: 1 + MKL_NUM_THREADS: 1 + OMP_NUM_THREADS: 1 + ASV_FACTOR: 1.5 + ASV_SKIP_SLOW: 1 + run: | + set -x + # ID this runner + asv machine --yes + echo "Baseline: ${{ github.event.pull_request.base.sha }} (${{ github.event.pull_request.base.label }})" + echo "Contender: ${GITHUB_SHA} (${{ github.event.pull_request.head.label }})" + # Use mamba for env creation + # export CONDA_EXE=$(which mamba) + export CONDA_EXE=$(which conda) + # Run benchmarks for current commit against base + ASV_OPTIONS="--split --show-stderr --factor $ASV_FACTOR" + asv continuous $ASV_OPTIONS ${{ github.event.pull_request.base.sha }} ${GITHUB_SHA} \ + | sed "/Traceback \|failed$\|PERFORMANCE DECREASED/ s/^/::error::/" \ + | tee benchmarks.log + # Report and export results for subsequent steps + if grep "Traceback \|failed\|PERFORMANCE DECREASED" benchmarks.log > /dev/null ; then + exit 1 + fi + working-directory: ${{ env.ASV_DIR }} + + - name: Add instructions to artifact + if: always() + run: | + cp benchmarks/README_CI.md benchmarks.log .asv/results/ + working-directory: ${{ env.ASV_DIR }} + + - uses: actions/upload-artifact@v2 + if: always() + with: + name: asv-benchmark-results-${{ runner.os }} + path: ${{ env.ASV_DIR }}/.asv/results diff --git a/asv_bench/benchmarks/README_CI.md b/asv_bench/benchmarks/README_CI.md new file mode 100644 index 00000000000..9d86cc257ef --- /dev/null +++ b/asv_bench/benchmarks/README_CI.md @@ -0,0 +1,122 @@ +# Benchmark CI + + + + + +## How it works + +The `asv` suite can be run for any PR on GitHub Actions (check workflow `.github/workflows/benchmarks.yml`) by adding a `run-benchmark` label to said PR. This will trigger a job that will run the benchmarking suite for the current PR head (merged commit) against the PR base (usually `main`). + +We use `asv continuous` to run the job, which runs a relative performance measurement. This means that there's no state to be saved and that regressions are only caught in terms of performance ratio (absolute numbers are available but they are not useful since we do not use stable hardware over time). `asv continuous` will: + +* Compile `scikit-image` for _both_ commits. We use `ccache` to speed up the process, and `mamba` is used to create the build environments. +* Run the benchmark suite for both commits, _twice_ (since `processes=2` by default). +* Generate a report table with performance ratios: + * `ratio=1.0` -> performance didn't change. + * `ratio<1.0` -> PR made it slower. + * `ratio>1.0` -> PR made it faster. + +Due to the sensitivity of the test, we cannot guarantee that false positives are not produced. In practice, values between `(0.7, 1.5)` are to be considered part of the measurement noise. When in doubt, running the benchmark suite one more time will provide more information about the test being a false positive or not. + +## Running the benchmarks on GitHub Actions + +1. On a PR, add the label `run-benchmark`. +2. The CI job will be started. Checks will appear in the usual dashboard panel above the comment box. +3. If more commits are added, the label checks will be grouped with the last commit checks _before_ you added the label. +4. Alternatively, you can always go to the `Actions` tab in the repo and [filter for `workflow:Benchmark`](https://github.com/scikit-image/scikit-image/actions?query=workflow%3ABenchmark). Your username will be assigned to the `actor` field, so you can also filter the results with that if you need it. + +## The artifacts + +The CI job will also generate an artifact. This is the `.asv/results` directory compressed in a zip file. Its contents include: + +* `fv-xxxxx-xx/`. A directory for the machine that ran the suite. It contains three files: + * `.json`, `.json`: the benchmark results for each commit, with stats. + * `machine.json`: details about the hardware. +* `benchmarks.json`: metadata about the current benchmark suite. +* `benchmarks.log`: the CI logs for this run. +* This README. + +## Re-running the analysis + +Although the CI logs should be enough to get an idea of what happened (check the table at the end), one can use `asv` to run the analysis routines again. + +1. Uncompress the artifact contents in the repo, under `.asv/results`. This is, you should see `.asv/results/benchmarks.log`, not `.asv/results/something_else/benchmarks.log`. Write down the machine directory name for later. +2. Run `asv show` to see your available results. You will see something like this: + +``` +$> asv show + +Commits with results: + +Machine : Jaimes-MBP +Environment: conda-py3.9-cython-numpy1.20-scipy + + 00875e67 + +Machine : fv-az95-499 +Environment: conda-py3.7-cython-numpy1.17-pooch-scipy + + 8db28f02 + 3a305096 +``` + +3. We are interested in the commits for `fv-az95-499` (the CI machine for this run). We can compare them with `asv compare` and some extra options. `--sort ratio` will show largest ratios first, instead of alphabetical order. `--split` will produce three tables: improved, worsened, no changes. `--factor 1.5` tells `asv` to only complain if deviations are above a 1.5 ratio. `-m` is used to indicate the machine ID (use the one you wrote down in step 1). Finally, specify your commit hashes: baseline first, then contender! + +``` +$> asv compare --sort ratio --split --factor 1.5 -m fv-az95-499 8db28f02 3a305096 + +Benchmarks that have stayed the same: + + before after ratio + [8db28f02] [3a305096] + + n/a n/a n/a benchmark_restoration.RollingBall.time_rollingball_ndim + 1.23±0.04ms 1.37±0.1ms 1.12 benchmark_transform_warp.WarpSuite.time_to_float64(, 128, 3) + 5.07±0.1μs 5.59±0.4μs 1.10 benchmark_transform_warp.ResizeLocalMeanSuite.time_resize_local_mean(, (192, 192, 192), (192, 192, 192)) + 1.23±0.02ms 1.33±0.1ms 1.08 benchmark_transform_warp.WarpSuite.time_same_type(, 128, 3) + 9.45±0.2ms 10.1±0.5ms 1.07 benchmark_rank.Rank3DSuite.time_3d_filters('majority', (32, 32, 32)) + 23.0±0.9ms 24.6±1ms 1.07 benchmark_interpolation.InterpolationResize.time_resize((80, 80, 80), 0, 'symmetric', , True) + 38.7±1ms 41.1±1ms 1.06 benchmark_transform_warp.ResizeLocalMeanSuite.time_resize_local_mean(, (2048, 2048), (192, 192, 192)) + 4.97±0.2μs 5.24±0.2μs 1.05 benchmark_transform_warp.ResizeLocalMeanSuite.time_resize_local_mean(, (2048, 2048), (2048, 2048)) + 4.21±0.2ms 4.42±0.3ms 1.05 benchmark_rank.Rank3DSuite.time_3d_filters('gradient', (32, 32, 32)) + +... +``` + +If you want more details on a specific test, you can use `asv show`. Use `-b pattern` to filter which tests to show, and then specify a commit hash to inspect: + +``` +$> asv show -b time_to_float64 8db28f02 + +Commit: 8db28f02 + +benchmark_transform_warp.WarpSuite.time_to_float64 [fv-az95-499/conda-py3.7-cython-numpy1.17-pooch-scipy] + ok + =============== ============= ========== ============= ========== ============ ========== ============ ========== ============ + -- N / order + --------------- -------------------------------------------------------------------------------------------------------------- + dtype_in 128 / 0 128 / 1 128 / 3 1024 / 0 1024 / 1 1024 / 3 4096 / 0 4096 / 1 4096 / 3 + =============== ============= ========== ============= ========== ============ ========== ============ ========== ============ + numpy.uint8 2.56±0.09ms 523±30μs 1.28±0.05ms 130±3ms 28.7±2ms 81.9±3ms 2.42±0.01s 659±5ms 1.48±0.01s + numpy.uint16 2.48±0.03ms 530±10μs 1.28±0.02ms 130±1ms 30.4±0.7ms 81.1±2ms 2.44±0s 653±3ms 1.47±0.02s + numpy.float32 2.59±0.1ms 518±20μs 1.27±0.01ms 127±3ms 26.6±1ms 74.8±2ms 2.50±0.01s 546±10ms 1.33±0.02s + numpy.float64 2.48±0.04ms 513±50μs 1.23±0.04ms 134±3ms 30.7±2ms 85.4±2ms 2.55±0.01s 632±4ms 1.45±0.01s + =============== ============= ========== ============= ========== ============ ========== ============ ========== ============ + started: 2021-07-06 06:14:36, duration: 1.99m +``` + +## Other details + +### Skipping slow or demanding tests + +To minimize the time required to run the full suite, we trimmed the parameter matrix in some cases and, in others, directly skipped tests that ran for too long or require too much memory. Unlike `pytest`, `asv` does not have a notion of marks. However, you can `raise NotImplementedError` in the setup step to skip a test. In that vein, a new private function is defined at `benchmarks.__init__`: `_skip_slow`. This will check if the `ASV_SKIP_SLOW` environment variable has been defined. If set to `1`, it will raise `NotImplementedError` and skip the test. To implement this behavior in other tests, you can add the following attribute: + +```python +from . import _skip_slow # this function is defined in benchmarks.__init__ + +def time_something_slow(): + pass + +time_something.setup = _skip_slow +``` diff --git a/asv_bench/benchmarks/__init__.py b/asv_bench/benchmarks/__init__.py index b0adb2feafd..02c3896e236 100644 --- a/asv_bench/benchmarks/__init__.py +++ b/asv_bench/benchmarks/__init__.py @@ -1,4 +1,5 @@ import itertools +import os import numpy as np @@ -46,3 +47,21 @@ def randint(low, high=None, size=None, frac_minus=None, seed=0): x.flat[inds] = -1 return x + + +def _skip_slow(): + """ + Use this function to skip slow or highly demanding tests. + + Use it as a `Class.setup` method or a `function.setup` attribute. + + Examples + -------- + >>> from . import _skip_slow + >>> def time_something_slow(): + ... pass + ... + >>> time_something.setup = _skip_slow + """ + if os.environ.get("ASV_SKIP_SLOW", "0") == "1": + raise NotImplementedError("Skipping this test...") diff --git a/asv_bench/benchmarks/combine.py b/asv_bench/benchmarks/combine.py index 308ca2afda4..a4f8db2786b 100644 --- a/asv_bench/benchmarks/combine.py +++ b/asv_bench/benchmarks/combine.py @@ -9,7 +9,7 @@ class Combine: def setup(self): """Create 4 datasets with two different variables""" - t_size, x_size, y_size = 100, 900, 800 + t_size, x_size, y_size = 50, 450, 400 t = np.arange(t_size) data = np.random.randn(t_size, x_size, y_size) diff --git a/asv_bench/benchmarks/dataarray_missing.py b/asv_bench/benchmarks/dataarray_missing.py index d79d2558b35..f89fe7f8eb9 100644 --- a/asv_bench/benchmarks/dataarray_missing.py +++ b/asv_bench/benchmarks/dataarray_missing.py @@ -2,12 +2,7 @@ import xarray as xr -from . import randn, requires_dask - -try: - import dask # noqa: F401 -except ImportError: - pass +from . import parameterized, randn, requires_dask def make_bench_data(shape, frac_nan, chunks): @@ -21,54 +16,65 @@ def make_bench_data(shape, frac_nan, chunks): return da -def time_interpolate_na(shape, chunks, method, limit): - if chunks is not None: - requires_dask() - da = make_bench_data(shape, 0.1, chunks=chunks) - actual = da.interpolate_na(dim="time", method="linear", limit=limit) - - if chunks is not None: - actual = actual.compute() - - -time_interpolate_na.param_names = ["shape", "chunks", "method", "limit"] -time_interpolate_na.params = ( - [(3650, 200, 400), (100, 25, 25)], - [None, {"x": 25, "y": 25}], - ["linear", "spline", "quadratic", "cubic"], - [None, 3], -) - - -def time_ffill(shape, chunks, limit): - - da = make_bench_data(shape, 0.1, chunks=chunks) - actual = da.ffill(dim="time", limit=limit) - - if chunks is not None: - actual = actual.compute() - - -time_ffill.param_names = ["shape", "chunks", "limit"] -time_ffill.params = ( - [(3650, 200, 400), (100, 25, 25)], - [None, {"x": 25, "y": 25}], - [None, 3], -) - - -def time_bfill(shape, chunks, limit): - - da = make_bench_data(shape, 0.1, chunks=chunks) - actual = da.bfill(dim="time", limit=limit) - - if chunks is not None: - actual = actual.compute() - - -time_bfill.param_names = ["shape", "chunks", "limit"] -time_bfill.params = ( - [(3650, 200, 400), (100, 25, 25)], - [None, {"x": 25, "y": 25}], - [None, 3], -) +def requires_bottleneck(): + try: + import bottleneck # noqa: F401 + except ImportError: + raise NotImplementedError() + + +class DataArrayMissingInterpolateNA: + def setup(self, shape, chunks, limit): + if chunks is not None: + requires_dask() + self.da = make_bench_data(shape, 0.1, chunks) + + @parameterized( + ["shape", "chunks", "limit"], + ( + [(365, 75, 75)], + [None, {"x": 25, "y": 25}], + [None, 3], + ), + ) + def time_interpolate_na(self, shape, chunks, limit): + actual = self.da.interpolate_na(dim="time", method="linear", limit=limit) + + if chunks is not None: + actual = actual.compute() + + +class DataArrayMissingBottleneck: + def setup(self, shape, chunks, limit): + requires_bottleneck() + if chunks is not None: + requires_dask() + self.da = make_bench_data(shape, 0.1, chunks) + + @parameterized( + ["shape", "chunks", "limit"], + ( + [(365, 75, 75)], + [None, {"x": 25, "y": 25}], + [None, 3], + ), + ) + def time_ffill(self, shape, chunks, limit): + actual = self.da.ffill(dim="time", limit=limit) + + if chunks is not None: + actual = actual.compute() + + @parameterized( + ["shape", "chunks", "limit"], + ( + [(365, 75, 75)], + [None, {"x": 25, "y": 25}], + [None, 3], + ), + ) + def time_bfill(self, shape, chunks, limit): + actual = self.da.ffill(dim="time", limit=limit) + + if chunks is not None: + actual = actual.compute() diff --git a/asv_bench/benchmarks/dataset_io.py b/asv_bench/benchmarks/dataset_io.py index e99911d752c..6c2e15c54e9 100644 --- a/asv_bench/benchmarks/dataset_io.py +++ b/asv_bench/benchmarks/dataset_io.py @@ -5,7 +5,7 @@ import xarray as xr -from . import randint, randn, requires_dask +from . import _skip_slow, randint, randn, requires_dask try: import dask @@ -28,6 +28,9 @@ class IOSingleNetCDF: number = 5 def make_ds(self): + # TODO: Lazily skipped in CI as it is very demanding and slow. + # Improve times and remove errors. + _skip_slow() # single Dataset self.ds = xr.Dataset() @@ -227,6 +230,9 @@ class IOMultipleNetCDF: number = 5 def make_ds(self, nfiles=10): + # TODO: Lazily skipped in CI as it is very demanding and slow. + # Improve times and remove errors. + _skip_slow() # multiple Dataset self.ds = xr.Dataset() @@ -429,6 +435,10 @@ def time_open_dataset_scipy_with_time_chunks(self): def create_delayed_write(): import dask.array as da + # TODO: Lazily skipped in CI as it is very demanding and slow. + # Improve times and remove errors. + _skip_slow() + vals = da.random.random(300, chunks=(1,)) ds = xr.Dataset({"vals": (["a"], vals)}) return ds.to_netcdf("file.nc", engine="netcdf4", compute=False) @@ -453,6 +463,11 @@ def setup(self): import distributed except ImportError: raise NotImplementedError() + + # TODO: Lazily skipped in CI as it is very demanding and slow. + # Improve times and remove errors. + _skip_slow() + self.client = distributed.Client() self.write = create_delayed_write() diff --git a/asv_bench/benchmarks/indexing.py b/asv_bench/benchmarks/indexing.py index 859c41c913d..15212ec0c61 100644 --- a/asv_bench/benchmarks/indexing.py +++ b/asv_bench/benchmarks/indexing.py @@ -5,11 +5,11 @@ import xarray as xr -from . import randint, randn, requires_dask +from . import parameterized, randint, randn, requires_dask -nx = 3000 -ny = 2000 -nt = 1000 +nx = 2000 +ny = 1000 +nt = 500 basic_indexes = { "1slice": {"x": slice(0, 3)}, @@ -21,7 +21,7 @@ "1slice": xr.DataArray(randn((3, ny), frac_nan=0.1), dims=["x", "y"]), "1slice-1scalar": xr.DataArray(randn(int(ny / 3) + 1, frac_nan=0.1), dims=["y"]), "2slicess-1scalar": xr.DataArray( - randn(int((nx - 6) / 3), frac_nan=0.1), dims=["x"] + randn(np.empty(nx)[slice(3, -3, 3)].size, frac_nan=0.1), dims=["x"] ), } @@ -51,7 +51,7 @@ } vectorized_assignment_values = { - "1-1d": xr.DataArray(randn((400, 2000)), dims=["a", "y"], coords={"a": randn(400)}), + "1-1d": xr.DataArray(randn((400, ny)), dims=["a", "y"], coords={"a": randn(400)}), "2-1d": xr.DataArray(randn(400), dims=["a"], coords={"a": randn(400)}), "3-2d": xr.DataArray( randn((4, 100)), dims=["a", "b"], coords={"a": randn(4), "b": randn(100)} @@ -77,50 +77,38 @@ def setup(self, key): class Indexing(Base): + @parameterized(["key"], [list(basic_indexes.keys())]) def time_indexing_basic(self, key): self.ds.isel(**basic_indexes[key]).load() - time_indexing_basic.param_names = ["key"] - time_indexing_basic.params = [list(basic_indexes.keys())] - + @parameterized(["key"], [list(outer_indexes.keys())]) def time_indexing_outer(self, key): self.ds.isel(**outer_indexes[key]).load() - time_indexing_outer.param_names = ["key"] - time_indexing_outer.params = [list(outer_indexes.keys())] - + @parameterized(["key"], [list(vectorized_indexes.keys())]) def time_indexing_vectorized(self, key): self.ds.isel(**vectorized_indexes[key]).load() - time_indexing_vectorized.param_names = ["key"] - time_indexing_vectorized.params = [list(vectorized_indexes.keys())] - class Assignment(Base): + @parameterized(["key"], [list(basic_indexes.keys())]) def time_assignment_basic(self, key): ind = basic_indexes[key] val = basic_assignment_values[key] self.ds["var1"][ind.get("x", slice(None)), ind.get("y", slice(None))] = val - time_assignment_basic.param_names = ["key"] - time_assignment_basic.params = [list(basic_indexes.keys())] - + @parameterized(["key"], [list(outer_indexes.keys())]) def time_assignment_outer(self, key): ind = outer_indexes[key] val = outer_assignment_values[key] self.ds["var1"][ind.get("x", slice(None)), ind.get("y", slice(None))] = val - time_assignment_outer.param_names = ["key"] - time_assignment_outer.params = [list(outer_indexes.keys())] - + @parameterized(["key"], [list(vectorized_indexes.keys())]) def time_assignment_vectorized(self, key): ind = vectorized_indexes[key] val = vectorized_assignment_values[key] self.ds["var1"][ind.get("x", slice(None)), ind.get("y", slice(None))] = val - time_assignment_vectorized.param_names = ["key"] - time_assignment_vectorized.params = [list(vectorized_indexes.keys())] - class IndexingDask(Indexing): def setup(self, key): diff --git a/asv_bench/benchmarks/interp.py b/asv_bench/benchmarks/interp.py index cded900ebbc..4b6691bcc0a 100644 --- a/asv_bench/benchmarks/interp.py +++ b/asv_bench/benchmarks/interp.py @@ -5,21 +5,17 @@ from . import parameterized, randn, requires_dask -nx = 3000 -long_nx = 30000000 -ny = 2000 -nt = 1000 -window = 20 +nx = 1500 +ny = 1000 +nt = 500 randn_xy = randn((nx, ny), frac_nan=0.1) randn_xt = randn((nx, nt)) randn_t = randn((nt,)) -randn_long = randn((long_nx,), frac_nan=0.1) - new_x_short = np.linspace(0.3 * nx, 0.7 * nx, 100) -new_x_long = np.linspace(0.3 * nx, 0.7 * nx, 1000) -new_y_long = np.linspace(0.1, 0.9, 1000) +new_x_long = np.linspace(0.3 * nx, 0.7 * nx, 500) +new_y_long = np.linspace(0.1, 0.9, 500) class Interpolation: diff --git a/asv_bench/benchmarks/pandas.py b/asv_bench/benchmarks/pandas.py index 42ef18ac0c2..8aaa515d417 100644 --- a/asv_bench/benchmarks/pandas.py +++ b/asv_bench/benchmarks/pandas.py @@ -1,6 +1,8 @@ import numpy as np import pandas as pd +import xarray as xr + from . import parameterized @@ -20,5 +22,5 @@ def setup(self, dtype, subset): self.series = series @parameterized(["dtype", "subset"], ([int, float], [True, False])) - def time_to_xarray(self, dtype, subset): - self.series.to_xarray() + def time_from_series(self, dtype, subset): + xr.DataArray.from_series(self.series) diff --git a/asv_bench/benchmarks/reindexing.py b/asv_bench/benchmarks/reindexing.py index fe4fa500c09..9d0767fc3b3 100644 --- a/asv_bench/benchmarks/reindexing.py +++ b/asv_bench/benchmarks/reindexing.py @@ -4,38 +4,42 @@ from . import requires_dask +ntime = 500 +nx = 50 +ny = 50 + class Reindex: def setup(self): - data = np.random.RandomState(0).randn(1000, 100, 100) + data = np.random.RandomState(0).randn(ntime, nx, ny) self.ds = xr.Dataset( {"temperature": (("time", "x", "y"), data)}, - coords={"time": np.arange(1000), "x": np.arange(100), "y": np.arange(100)}, + coords={"time": np.arange(ntime), "x": np.arange(nx), "y": np.arange(ny)}, ) def time_1d_coarse(self): - self.ds.reindex(time=np.arange(0, 1000, 5)).load() + self.ds.reindex(time=np.arange(0, ntime, 5)).load() def time_1d_fine_all_found(self): - self.ds.reindex(time=np.arange(0, 1000, 0.5), method="nearest").load() + self.ds.reindex(time=np.arange(0, ntime, 0.5), method="nearest").load() def time_1d_fine_some_missing(self): self.ds.reindex( - time=np.arange(0, 1000, 0.5), method="nearest", tolerance=0.1 + time=np.arange(0, ntime, 0.5), method="nearest", tolerance=0.1 ).load() def time_2d_coarse(self): - self.ds.reindex(x=np.arange(0, 100, 2), y=np.arange(0, 100, 2)).load() + self.ds.reindex(x=np.arange(0, nx, 2), y=np.arange(0, ny, 2)).load() def time_2d_fine_all_found(self): self.ds.reindex( - x=np.arange(0, 100, 0.5), y=np.arange(0, 100, 0.5), method="nearest" + x=np.arange(0, nx, 0.5), y=np.arange(0, ny, 0.5), method="nearest" ).load() def time_2d_fine_some_missing(self): self.ds.reindex( - x=np.arange(0, 100, 0.5), - y=np.arange(0, 100, 0.5), + x=np.arange(0, nx, 0.5), + y=np.arange(0, ny, 0.5), method="nearest", tolerance=0.1, ).load() diff --git a/asv_bench/benchmarks/repr.py b/asv_bench/benchmarks/repr.py index 405f6cd0530..4bf2ace352d 100644 --- a/asv_bench/benchmarks/repr.py +++ b/asv_bench/benchmarks/repr.py @@ -28,9 +28,9 @@ def time_repr_html(self): class ReprMultiIndex: def setup(self): index = pd.MultiIndex.from_product( - [range(10000), range(10000)], names=("level_0", "level_1") + [range(1000), range(1000)], names=("level_0", "level_1") ) - series = pd.Series(range(100000000), index=index) + series = pd.Series(range(1000 * 1000), index=index) self.da = xr.DataArray(series) def time_repr(self): diff --git a/asv_bench/benchmarks/rolling.py b/asv_bench/benchmarks/rolling.py index 93c3c6aed4e..f0e18bf2153 100644 --- a/asv_bench/benchmarks/rolling.py +++ b/asv_bench/benchmarks/rolling.py @@ -5,10 +5,10 @@ from . import parameterized, randn, requires_dask -nx = 3000 -long_nx = 30000000 -ny = 2000 -nt = 1000 +nx = 300 +long_nx = 30000 +ny = 200 +nt = 100 window = 20 randn_xy = randn((nx, ny), frac_nan=0.1) @@ -44,21 +44,21 @@ def time_rolling(self, func, center): def time_rolling_long(self, func, pandas): if pandas: se = self.da_long.to_series() - getattr(se.rolling(window=window), func)() + getattr(se.rolling(window=window, min_periods=window), func)() else: - getattr(self.da_long.rolling(x=window), func)().load() + getattr(self.da_long.rolling(x=window, min_periods=window), func)().load() - @parameterized(["window_", "min_periods"], ([20, 40], [5, None])) + @parameterized(["window_", "min_periods"], ([20, 40], [5, 5])) def time_rolling_np(self, window_, min_periods): self.ds.rolling(x=window_, center=False, min_periods=min_periods).reduce( - getattr(np, "nanmean") + getattr(np, "nansum") ).load() - @parameterized(["center", "stride"], ([True, False], [1, 200])) + @parameterized(["center", "stride"], ([True, False], [1, 1])) def time_rolling_construct(self, center, stride): self.ds.rolling(x=window, center=center).construct( "window_dim", stride=stride - ).mean(dim="window_dim").load() + ).sum(dim="window_dim").load() class RollingDask(Rolling): diff --git a/asv_bench/benchmarks/unstacking.py b/asv_bench/benchmarks/unstacking.py index 8d0c3932870..2c5b7ca7821 100644 --- a/asv_bench/benchmarks/unstacking.py +++ b/asv_bench/benchmarks/unstacking.py @@ -7,7 +7,7 @@ class Unstacking: def setup(self): - data = np.random.RandomState(0).randn(500, 1000) + data = np.random.RandomState(0).randn(250, 500) self.da_full = xr.DataArray(data, dims=list("ab")).stack(flat_dim=[...]) self.da_missing = self.da_full[:-1] self.df_missing = self.da_missing.to_pandas() @@ -26,4 +26,4 @@ class UnstackingDask(Unstacking): def setup(self, *args, **kwargs): requires_dask() super().setup(**kwargs) - self.da_full = self.da_full.chunk({"flat_dim": 50}) + self.da_full = self.da_full.chunk({"flat_dim": 25}) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index ae66f9d33ae..26702680d5d 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -113,6 +113,8 @@ Internal Changes dask versions of functions when acting on dask arrays, instead relying numpy and dask's adherence to NEP-18 to dispatch automatically. (:pull:`5571`) By `Tom Nicholas `_. +- Add an ASV benchmark CI and improve performance of the benchmarks (:pull:`5796`) + By `Jimmy Westling `_. .. _whats-new.0.19.0: From fdabf3bea5c750939a4a2ae60f80ed34a6aebd58 Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Sun, 24 Oct 2021 13:35:42 +0200 Subject: [PATCH 5/5] Only run asv benchmark when labeled (#5893) * Only run benchmark when labeled * Update benchmarks.yml * Update benchmarks.yml --- .github/workflows/benchmarks.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index 7c3cb114201..de506546ac9 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -7,10 +7,7 @@ on: jobs: benchmark: - if: | - ${{ contains( github.event.pull_request.labels.*.name, 'run-benchmark') - && github.event_name == 'pull_request' - || github.event_name == 'workflow_dispatch' }} + if: ${{ contains( github.event.pull_request.labels.*.name, 'run-benchmark') && github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' }} name: Linux runs-on: ubuntu-20.04 env: