Conversation
…outside the read domain
This branch has not been deployed
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
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 raisedRestriction 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_imagereturns the smallest hypercube containing all in-range entries, holes included._hyperslicenow 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 frominverse_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.__debug__(module flag_FILL_GATHER_HOLES, patchable in tests) float results get NaN at hole points. Withpython -Othe fill is skipped and hole points have unspecified values.inverse_imagecould narrow aLOCALdimension, and a followingneighbor_sumsilently summed the wrong neighbours (e.g. a C2E sum over an edge field missing two of three neighbours returned3 * z[e2]). ALOCALdimension may now be narrowed only if every dropped entry is the skip value; otherwiseinverse_imageraisesRestriction 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
mainand overlaps #2895.git merge-treeagainst #2895's head (c654d8b) shows two conflicts, both insrc/gt4py/next/embedded/nd_array_field.py:NdArrayConnectivityField.inverse_image: feat[next]: JAX support for connectivities and premap #2895 replaces the_hyperslicecall withself._image_slices(...). The merged version must returnhas_holes=Falsefrom the_image_slicesshortcut, fall through to_hypersliceotherwise, and keep the local-dimension check._gather_premap.take_index: feat[next]: JAX support for connectivities and premap #2895 passesskip_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