Skip to content

fix[next]: embedded gathers accept out-of-range connectivity entries outside the read domain - #2916

Draft
havogt wants to merge 1 commit into
GridTools:mainfrom
havogt:f52-embedded-holes
Draft

havogt wants to merge 1 commit into
GridTools:mainfrom
havogt:f52-embedded-holes

Conversation

@havogt

@havogt havogt commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Description

Embedded gathers (neighbour shifts, as_offset) used to refuse any connectivity whose in-range entries do not form a clean hypercube: one out-of-range, non-skip entry ("hole") inside the bounding box raised Restriction generates non-contiguous or empty dimensions., even if the program never reads that point. This happens e.g. with a C2E gather of an edge intermediate whose domain lost the leading boundary edges: the hole cells lie outside the program domain, but the gather still raised.

Changes in embedded/nd_array_field.py:

  • NdArrayConnectivityField.inverse_image returns the smallest hypercube containing all in-range entries, holes included. _hyperslice now returns (slices, has_holes); an empty image still raises (Restriction generates an empty domain.).
  • _gather_premap: only for a connectivity whose hypercube has holes (cached flag from inverse_image, so gathers without holes do no extra work), hole indices are redirected to the data domain start so the gather never indexes out of bounds. Skip entries are left as today.
  • Under __debug__ (module flag _FILL_GATHER_HOLES, patchable in tests) float results get NaN at hole points. With python -O the fill is skipped and hole points have unspecified values.
  • Non-float dtypes (int, bool): no fill in either mode and no error; hole points have unspecified values (currently the value at the domain start). Raising in debug mode only would reject programs that never read the holes, and only in debug mode, so that option was not taken.
  • Local dimensions are never narrowed around non-skip entries. Previously, when only some neighbour columns were in range, inverse_image could narrow a LOCAL dimension, and a following neighbor_sum silently summed the wrong neighbours (e.g. a C2E sum over an edge field missing two of three neighbours returned 3 * z[e2]). A LOCAL dimension may now be narrowed only if every dropped entry is the skip value; otherwise inverse_image raises Restriction generates a partial local dimension ....

Known limitations of the NaN fill: NaN only marks a hole while every op on the way propagates it. A comparison (where(z(C2E) > 0, ...)) or an int cast turns it into finite values. With the fill off, hole reads are never flagged.

Existing unit tests that asserted "holes raise" now assert the new contract (bounding box, NaN at holes under the flag): test_premap_non_contiguous_inverse_image_raises (→ ..._has_holes), test_as_offset_scattered_oob_raises (→ ..._has_holes), test_connectivity_field_inverse_image_2d_domain, test_connectivity_field_inverse_image_non_contiguous, test_connectivity_field_inverse_image_2d_domain_skip_values, test_hyperslice (new return type, plus two cases).

Overlap with #2895

This PR is based on main and overlaps #2895. git merge-tree against #2895's head (c654d8b) shows two conflicts, both in src/gt4py/next/embedded/nd_array_field.py:

  1. NdArrayConnectivityField.inverse_image: feat[next]: JAX support for connectivities and premap #2895 replaces the _hyperslice call with self._image_slices(...). The merged version must return has_holes=False from the _image_slices shortcut, fall through to _hyperslice otherwise, and keep the local-dimension check.
  2. _gather_premap.take_index: feat[next]: JAX support for connectivities and premap #2895 passes skip_value_replacement=start, this PR adds the hole redirection and fill. The resolution keeps both: skips → start (from feat[next]: JAX support for connectivities and premap #2895), holes → start + fill (from this PR).

The test files merge cleanly.

Requirements

  • All fixes and/or new features come with corresponding tests.
  • Important design decisions have been documented in the appropriate ADR inside the docs/development/ADRs/ folder.

This branch has not been deployed

No deployments
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.

1 participant