fix: dask divide by zero error when empty dimensions are passed into partial autochunk spec - #11486
Open
charles-turner-1 wants to merge 17 commits into
Open
fix: dask divide by zero error when empty dimensions are passed into partial autochunk spec#11486charles-turner-1 wants to merge 17 commits into
charles-turner-1 wants to merge 17 commits into
Conversation
…nally fill in zero dimensions with auto - otherwise dask gives a(n avoidable) divide by zero error
|
Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient. |
for more information, see https://pre-commit.ci
charles-turner-1
marked this pull request as draft
July 31, 2026 08:43
Contributor
Author
|
I've had Claude Opus 5 take a look at my changes with an eye to finding any lurking bugs or completeness issues. It's suggestion is to push these changes down into namedarray, which I'm currently investigating - looks like the current implementation doesn't generalise well - still raises if I'll undraft this when I'm satisfied with my changes. |
…::NamedArray.chunk` was the right shout. Haven't updated tests yet
…er-1/xarray into empty-partial-chunk
* Add myst_nb * Migrate doc/getting-started-guide/quick-overview.md * Migrate doc/get-help/faq.md * Migrate doc/internals/zarr-encoding-spec.md * Migrate doc/user-guide/hierarchical-data.md * Migrate doc/user-guide/reshaping.md * Migrate doc/user-guide/plotting.md * Migrate doc/user-guide/interpolation.md * Migrate doc/user-guide/combining.md * Migrate doc/user-guide/groupby.md * Fix references * Migrate doc/user-guide/weather-climate.md * Migrate doc/user-guide/testing.md * Migrate doc/user-guide/terminology.md * Fix cross-links * Migrate doc/user-guide/time-series.md * Migrate doc/user-guide/data-structures.md * Migrate doc/internals/internal-design.md * Migrate doc/internals/how-to-create-custom-index.md * Fix cross-links * Migrate doc/internals/duck-arrays-integration.md * Migrate doc/internals/time-coding.md * Fix cross-linking * Migrate doc/internals/extending-xarray.md * Fix path * Replace jupyter-input and jupyter-output with python code blocks and text blocks * Migrate whats-new.md code blocks * Migrate doc/internals/how-to-add-new-backend.rst * Migrate doc/user-guide/complex-numbers.md * Move from tabs to sections Doesn't look like myst-nb supports code cells in tabs executablebooks/MyST-NB#724 * Remove myst_parser Handled by myst_nb * Fix crosslinks * Migrate doc/user-guide/dask.md * Migrate doc/user-guide/duckarrays.md * Migrate doc/user-guide/indexing.md * Migrate doc/user-guide/io.md * Fix tab groups * Migrate doc/user-guide/pandas.md * Fix cross-linking and code * Replace `hide-input` with `remove-input` remove-input actually removes the input instead of putting it in a dropdown * Replace `hide-output` with `remove-output` remove-output actually removes the output instead of putting it in a dropdown * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix admonitions Do pre-commit separately * Fix admonition wrapping and make code blocks python * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * set 'nb_execution_timeout = 60' * Set nb_execution_excludepatterns instead * Fix path * Delete nbsphinx * Fix unnest code cells from admonitions Was getting the following errors: /Users/Hodgs004/coding/repos/xarray/doc/user-guide/plotting.md:392: WARNING: Found an unexpected `code-cell` or `raw-cell` directive. Either this file was not converted to a notebook, because Jupytext header content was missing, or the `code-cell` was not converted, because it is nested. See https://myst-nb.readthedocs.io/en/latest/authoring/text-notebooks.html for more information. [mystnb.nbcell] * Fix level headers and cross-references * Merge code cells These cells were visually merged in prior docs. * Format fix * Merge code blocks * Merge cells * fix links * Review copy edits * Fix formatting * Fix crossref * Remove nbsphinx config * Update whats-new * Remove nbsphinx metadata from notebook and update conf.py * Patch ROMS_ocean_model.ipynb metadata To extend execution time https://myst-nb.readthedocs.io/en/latest/computation/execute.html#execution-timeout --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
<details><summary>Claude's draft</summary> Report compression and filter settings from the h5netcdf backend in the variable ``encoding`` consistently with the netCDF4 backend, using h5netcdf's ``Variable.filters()``. Data compressed with codecs such as ``zstd`` or ``blosc`` now keeps its compression when re-saved instead of being silently written uncompressed. - ``_extract_nc4_variable_encoding`` translates the boolean netCDF4-style compression flags (zlib/szip/bzip2/blosc/zstd) produced by ``filters()`` into a single h5py-style ``compression`` string. When both styles are supplied, the explicit ``compression`` takes precedence. - Bump the minimum supported h5netcdf to 1.8.0 (which introduced these compatibility features) and add h5netcdf to the policy ``ignored_violations`` so the intentionally-newer pin does not fail the Minimum Version Policy check. - Drop the now-unused ``requires_h5netcdf_1_7_0_or_above`` test helper. - Document the change in whats-new.rst. xref: pydata#10657 Co-authored-by: Claude <noreply@anthropic.com> Resume this Claude session: ``` claude --resume 9faa34e9-aaf5-4e94-a2b7-b03f468175f8 ``` </details>
…er-1/xarray into empty-partial-chunk
charles-turner-1
left a comment
Contributor
Author
There was a problem hiding this comment.
to revert autoformatter
charles-turner-1
left a comment
Contributor
Author
There was a problem hiding this comment.
remove redundant test
charles-turner-1
marked this pull request as ready for review
July 31, 2026 10:27
charles-turner-1
added a commit
to charles-turner-1/xrexpr
that referenced
this pull request
Jul 31, 2026
The previous commit refused the hop by folding the schema inside the rule and sizing the select against it. That bought the fix at the cost of the package's "no optimiser rule reads a size" invariant, restated in four places. It is the wrong trade, and it was also incomplete. Emptiness is a fact about the *indexer*, not the schema: ``isel(lat=[])`` empties ``lat`` however long ``lat`` was. So the guard becomes ``_may_empty``, a ``match`` over ``Indexer`` closed with ``assert_never`` -- the third policy site over a value sum type -- and the rule reads nothing outside its two nodes. It takes the select's ``name`` as well as the value, for the reason ``schema._selected_size`` does: a ``sel`` slice names labels, so ``sel(lat=slice(0, 30))`` selects nothing on a dim whose labels start at 100. Repairing the hop instead was measured and does not work. Injecting ``"auto"`` for the emptied dim -- xarray's own fix in pydata/xarray#11486 -- still raises, because ``auto_chunks`` pins the empty dim to ``(0,)`` and *recurses*, and on the second pass it is no longer ``"auto"`` and so poisons ``largest_block``. Promoting every dim to ``"auto"`` fails the same way above ``limit ** (1 / ndim)``: 3-D, every dim "auto", lat empty: time=lon=100 ok time=lon=256 ZeroDivisionError time=lon=300 ZeroDivisionError -- 300x0x300, zero bytes of data Only dask's ``previous_chunks`` branch escapes the recursion, and a plan cannot know statically that the array is dask-backed. So the hop must stay refused. The same measurement found the half of #121 the previous fix missed. A uniform ``chunk("auto")`` does *not* take "a different path in dask" -- it takes the same one, and the small fixtures survived the bug rather than avoiding it, because ``auto_chunks`` pins every dim below ``limit ** (1 / ndim)`` and returns before it can divide. ``ds.plan.chunk("auto").isel(lat=[]).collect()`` raised where eager returned. ``_auto_sizing`` now classifies the uniform spec through the same ``classify_chunk``, so one taxonomy answers for both spellings. Costs nothing measurable: over 2000 generated chains -- 164 (Rechunk, Select) adjacencies, 99 pushable, 15 auto-sizing -- the indexer rule and the extent rule push the same 8, with zero per-case disagreements. They do differ in principle (``isel(time=slice(3, 8))`` is refused now and was allowed before), which ``test_a_mid_slice_is_a_barrier_at_an_auto_sizing_rechunk`` pins. Reverted with it: ``_survives_the_select``, the fold and trusted-prefix preamble, the module-docstring paragraph on extents, and the invariant as restated in ``SchemaState.sizes`` and ``test_dim_sizes``. ``test_rewrites_survive_unknown_dim_sizes`` loses its carve-out and is an unqualified property again, over rechunk chains too. New tests. ``_may_empty``'s soundness is checked against real xarray in ``test_indexers``, with an anti-vacuity twin. A whole-plan property asserts no *rule* creates the hazardous adjacency. The chain-equality property re-runs under ``array.chunk-size = "1kB"``, which is what makes the 1-5-length generated dims hazardous rather than vacuous -- the same setting the accessor regression uses, in place of a multi-hundred-megabyte fixture, because it names the condition the crash actually depends on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
I discovered this whilst playing with charles-turner-1/xrexpr#121 (expression rewriting).
TLDR; auto-chunking on zero length dimensions fails, because dask divides by zero. Same issue for Datasets & Variables:
I've hidden the traceback in a detail block because it's pretty long.
Details
I haven't opened an issue for this - I could see a straightforward fix so I just got cracking on it, but I did have a look and I couldn't find any extant issues mentioning it. Happy to open one for clarity!
I also stuck the test right below the chunking tests in the dataarray section - not totally sure it's the best place for it, open to suggestions!
Checklist
whats-new.rstapi.rstN/AAI Disclosure
I wrote all code in this myself, but had Claude Opus 5 review my changes a couple of times until I was satisfied its critiques weren't super helpful.