feat[next]: allow runtime shift offsets, and use them for the SWM periodic halo - #2750
Draft
havogt wants to merge 1 commit into
Draft
feat[next]: allow runtime shift offsets, and use them for the SWM periodic halo#2750havogt wants to merge 1 commit into
havogt wants to merge 1 commit into
Conversation
Cartesian shift offsets had to be literals, which meant a periodic wrap by a runtime grid size could not be written in the DSL. Relax the frontend to accept a non-literal integral offset, lower it, and canonicalize the resulting literal to an OffsetLiteral so domain inference sees a real offset rather than the unknown-access sentinel. Uses it in the shallow water example, whose halo update moves from NumPy into the field operator. Updates test_premap_cartesian_non_literal_offset, which pinned the old rejection, and adds the SUB and non-integral cases.
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.
Stacked on #2749. Moves the shallow water model's periodic halo update into the DSL, replacing the NumPy halo exchange that PR 1 deliberately used to stay free of
src/changes.The example side
timestepnow returns the wrapped prognostic fields, and the time loop touches no halo at all. The prognostic outputs are written over the full domain while the filtered levels only need the interior, so aprogramsupplies a per-outputdomain=.The rebinding order is load-bearing: each
concat_wherefeeds the next, so thejpass sees the already-i-corrected field and the corners come out right. A simultaneous formulation would leave them wrong.Why this needs frontend changes
The shift distance is
m, a program argument rather than a literal, and the frontend rejects that at FOAST construction:with_static_argscannot substitute — the check fires when the decorator runs at import, long beforecompile(m=[M])exists.Four files, and an ablation confirms none is redundant. Each row is measured by reverting that change alone and running all three backends:
ffront/foast_passes/type_deduction.pyffront/foast_to_gtir.pyiterator/transforms/normalize_shifts.py—CanonicalizeShiftOffsetspass_manager.py, wiring inapply_common_transformspass_manager.py, wiring inapply_fieldview_transformsThe last two lines are the non-obvious part: the two backends traverse different pass managers, so each wiring serves exactly one. Dropping either gives
AttributeError: 'DomainAccessDescriptor' object has no attribute 'grid_type'on the corresponding backend.CanonicalizeShiftOffsetsexists because our own lowering emits a shift whose offset is a runtime expression; oncemis inlined as a static argument it becomes anir.Literal, but domain inference expects anir.OffsetLiteraland otherwise yields the unknown-access sentinel.Validation
swm_reference.npz(NCAR data) — run manually, passespytest --nbmake examples— passesNumerically identical to PR 1: the NumPy reference is unchanged and still agrees, so moving the halo update into the DSL did not perturb the physics.
Unit suite
pytest tests/next_tests/unit_tests/compared against the same suite on #2749's branch, sincemainalready carries pre-existing failures:The one difference was
test_foast_to_gtir.py::test_premap_cartesian_non_literal_offset, which asserted thatinp(TDim + i)raises. That is the behaviour this PR deliberately changes, so the test is updated rather than silenced — it now pins the lowering, plus two new cases: theSUBform (negapplied to the offset) and that a non-literal float offset still errors, since a runtime offset has no fractional part to resolve to a staggered dimension.Failure sets are otherwise identical (
common the sorted lists shows no other difference in either direction).Note
An alternative that needs no
src/changes is to hardcode the wrap distance as a literal. I measured it and rejected it: it silently miscomputes at any--Mother than the default, because the domain follows the grid size but the wrap does not.